Update VS Code LLM integration

[blkt]

Summary

Replace obsolete Copilot settings with VS Code’s chatLanguageModels.json and customendpoint provider.

• Discover models from the authenticated gateway.
• Route requests through the ToolHive proxy.
• Support VS Code and VS Code Insiders.
• Preserve existing configuration and clean up obsolete settings.
• Reject or skip VS Code during lazy setup.

Fixes #6294

Type of change

• Bug fix

Test plan

• Build (task build)
• License validation (task license-check)
• Unit tests (task test) — changed packages pass; unrelated existing pkg/plugins/pluginsvc tests fail.
• Linting (task lint-fix) — blocked by an upstream nilness analyzer panic.

[codecov]

Codecov Report

❌ Patch coverage is 89.20188% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.22%. Comparing base (3b6e593) to head (a6d771b).

Files with missing lines	Patch %	Lines
pkg/client/llm_gateway_vscode.go	80.21%	18 Missing ⚠️
pkg/llm/setup.go	95.72%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6492      +/-   ##
==========================================
+ Coverage   78.19%   78.22%   +0.03%     
==========================================
  Files         769      770       +1     
  Lines       75078    75272     +194     
==========================================
+ Hits        58704    58883     +179     
- Misses      16369    16384      +15     
  Partials        5        5              

☔ 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.

[amirejaz]

Went through this against current main — it sits one commit on top, so the diff is clean.

The move from declarative key-patching to a dedicated writer is the right shape, and it follows what Claude Desktop and Codex already do rather than inventing a third mechanism. The provider-group patching looks careful — I traced the descending-index removal and it's correct. Two things I think need to change before this goes in, plus a question.

blocker: a failed /v1/models call aborts setup for every tool

Setup() runs discovery before anything is configured and returns the error straight up:

if hasVSCodeClient(detected) {
    discoveredModels, err = discoverGatewayModels(ctx, llmCfg)
    if err != nil {
        return err
    }
}

A 404, a brief network blip, or a body without "object":"list" means nothing gets configured at all — Claude Code, Cursor and Gemini included, none of which need discovery. Since #6481, VS Code is detected purely by its settings directory existing, so this hits anyone who has ever opened VS Code, even when they only came to set up Claude Code. A gateway that doesn't implement /v1/models can't run thv llm setup at all any more.

probeAnthropicPrefix is 50 lines up in the same file doing an equivalent gateway probe, and it swallows every failure and lets setup continue. I'd like discovery to degrade the same way — warn, drop VS Code from the detected set, configure the rest.

blocker: --models is ignored for VS Code

configureVSCode reads only cfg.DiscoveredModels; it never looks at cfg.Models, which is where --models lands. The flag's help text says to omit it "to use ... gateway model discovery", so passing it should mean don't discover. Right now the explicit list is discarded and the network call happens regardless. Wiring it through is also the natural escape hatch for the blocker above on gateways with no discovery endpoint.

question: has this been confirmed against a real VS Code?

Nothing on the test plan about manual testing, so asking rather than assuming. Every model gets:

RequestHeaders: map[string]string{"Authorization": "Bearer " + llmPlaceholderAPIKey},

The schema notes in #6294 list id, name, url, toolCalling, vision and maxOutputTokens, and describe apiKey as a secret field using an ${input:chat.lm.secret.<hash>} reference — requestHeaders isn't mentioned anywhere in there. If VS Code ignores it we've landed the same failure this PR exists to fix: config written, success reported, nothing actually routed. A screenshot of a ToolHive model answering a prompt in the picker would settle it.

suggestion: --lazy quietly stops working for VS Code

--client vscode --lazy now errors, and plain --lazy skips it with a warning. --lazy is documented for unattended/MDM provisioning, so VS Code can't be provisioned that way any more. Fair enough given discovery needs a token, but it's user-facing and there's no user-facing-change note on the PR — the settings.json → chatLanguageModels.json move deserves one too.

suggestion: duplication worth collapsing

discoverGatewayModels rebuilds probeAnthropicPrefix's transport setup almost verbatim — same DefaultTransport.Clone(), same nil guard, same InsecureSkipVerify toggle, same nolint pair. A shared gatewayHTTPClient(tlsSkipVerify bool) would cover both.

Separately, configureDetectedTools has no production callers left — Setup() goes straight to configureDetectedToolsWithDiscovery, and the wrapper only survives so five test call sites don't have to change. That's what the four unparam warnings are actually pointing at. I'd update those tests to pass nil and delete it rather than keep two entry points that will drift.

Tests

Unit coverage of the new writer is solid — idempotency, comment preservation, legacy-path migration, and the error table. Two gaps I'd want closed:

• The vscode/vscode-insider e2e cases were removed rather than migrated, so the new path has no end-to-end coverage at all. The table can express it — JSON Pointer supports array indices, so /0/vendor works against an array root. llmSettingsDirFor's vscode branches (lines 171 and 176) are dead now too.
• Nothing covers what happens to the other tools when discovery fails, which is how the first blocker slipped through.

Smaller things, take or leave: the response body isn't drained on the decode-failure path (it is on the non-200 path); model IDs are validated with TrimSpace but stored untrimmed, so " gpt-4" would go in with the space; the 128000/16000 token limits are applied to every model regardless of capability and could use a line noting they're placeholders the schema requires.

One note on the description — the failing lint run isn't a nilness panic, it's 12 ordinary findings (errcheck, gci, gocyclo, lll, revive, unparam) and most are auto-fixable. Flagging in case that claim is why lint-fix got skipped.

[stantheman0128]

Support. This is #6294 (chatLanguageModels.json / customendpoint), not a revert of #6481.

Empty LLMBinaryName for VS Code / Insiders / Cursor should still detect those GUI editors when they are installed and not on PATH. Skipping VS Code in --lazy is fine if model discovery needs a token.

I have not run thv llm setup against this branch.

[blkt]

Thanks for the thorough review.

The discovery and --models behavior follows the agreed requirements for #6294: VS Code models must come from authenticated /v1/models discovery, and discovery failures should return actionable errors instead of silently producing incomplete configuration. --models remains unrelated to VS Code discovery.

The requestHeaders structure was verified against current VS Code source/schema. Validation with a current real VS Code installation is still outstanding.

The old VS Code E2E cases covered the obsolete static settings.json integration. Replacing them meaningfully would require an authenticated gateway fixture serving /v1/models; the new stable and Insiders paths are currently covered by focused unit tests.

[amirejaz]

Thanks for the quick turnaround — most of this got picked up properly.

Confirmed fixed: gatewayHTTPClient shared by both probes, configureDetectedTools gone along with its four unparam warnings, Setup split into prepareSetupConfig/setupClients, body drained on the decode path, model IDs trimmed before dedup, token-limit constants documented, receiver and line-length fixes. Lint is down from 18 issues to 1, and the changed packages pass locally for me.

blocker: discovery failure still aborts setup for tools that don't need discovery

I think we're talking slightly past each other here. Agreed that VS Code should fail loudly rather than write a provider group with no models — configureVSCode already does that, and it's right.

My concern is the blast radius:

if hasVSCodeClient(detected) {
    discoveredModels, err = discoverGatewayModels(ctx, llmCfg)
    if err != nil {
        return err
    }
}

Claude Code's config isn't incomplete when /v1/models fails — it has no relationship to that endpoint. So a gateway without a discovery endpoint, or a transient blip, stops thv llm setup from configuring Claude Code, Cursor and Gemini too. Since #6481 VS Code is detected by settings-directory presence alone, so this catches people who never intended to configure it.

Two precedents for the fix are already in this PR:

• filterLazyVSCodeClients draws exactly the distinction I'd want — hard error when VS Code is the explicit --client target, warn-and-skip when it was only auto-detected. The same shape works here.
• After this push discoverGatewayModels and probeAnthropicPrefix share gatewayHTTPClient, so the two gateway probes now sit side by side with opposite failure semantics — one degrades, one aborts everything.

Hard error on --client vscode is right. It's the implicit multi-client path I'd like to see degrade.

suggestion: say something when --models is passed for VS Code

Taking your point that --models is unrelated to VS Code discovery. But it's currently accepted and silently discarded there, while being honoured for Claude Desktop and the Bedrock tiers. If it's intentionally unsupported, a warning would save someone the confusion.

Accepting the other two

requestHeaders verified against current source/schema is a fair answer. The outstanding real-install check is the last bit of #6294-shaped risk, so worth closing before or soon after merge.

On E2E, I checked and your claim holds — there's no httptest or gateway stub anywhere in that harness, so a meaningful replacement really would need new fixture machinery. Worth a follow-up issue so it doesn't quietly disappear.

On the lint bump — flexible, not blocking

Flagging this as a judgment call rather than a request. The bump pulled in nine //nolint:staticcheck suppressions across operator controllers to get green. The test-file ones are fine — they deliberately exercise deprecated fields. The production-controller ones defer a real Result.Requeue migration, and that's a decision that'd probably get more scrutiny in its own PR than as collateral in a VS Code change. Same for the remaining gci failure in pkg/authserver/spiffe_trust_test.go, which this PR doesn't otherwise touch.

Splitting lint.yml out would decouple the two, but if you'd rather keep them together to avoid a dependency between PRs that's reasonable as well — your call, I won't hold the review on it.

Unrelated to you: Go Vulnerability Check is red on go-git (GO-2026-6354/GO-2026-6355) reached through pkg/git. Not from this PR, and those IDs aren't in the IGNORED_VULNS list in security-scan.yml — needs its own fix on main.

[stantheman0128]

The 12:37 commit is the leftover from #6481.

discoverVSCodeModelsForSetup now matches filterLazyVSCodeClients: explicit --client vscode still fails loud when /v1/models dies; auto-detected VS Code (settings directory, empty LLMBinaryName) warns and drops so Cursor / Claude Code / Gemini still get configured. The new setup_test cases pin that.

Not a revert of #6481. I have not rerun thv llm setup on a6d771.