diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 473e663383..12597d211c 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -19,7 +19,7 @@ jobs: uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: # go.mod's directive, not 'stable': golangci-lint is pinned below - # v2.13.0 (see the version comment further down), and that older + # (see the version comment further down), and the pinned # release's bundled type-checker cannot parse the newer Go # toolchain's stdlib (e.g. a generic-method signature in # math/rand/v2 under Go 1.27), so linting must run on the Go @@ -45,12 +45,9 @@ jobs: ${{ runner.os }}-go-build- - name: Run golangci-lint - uses: golangci/golangci-lint-action@82606bf257cbaff209d206a39f5134f0cfbfd2ee # v9.2.1 + uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 with: - # Pin to a known-good release: v2.13.0 bundles honnef.co/go/tools - # v0.8.0-rc.1, whose nilness analyzer panics ("unhandled builtin - # recover") analyzing github.com/getsentry/sentry-go. Unpin once - # upstream ships a fixed release. - version: v2.12.2 + # Pin to a known-good release so lint behavior is reproducible. + version: v2.13.2 # Enable golangci-lint's built-in caching (removes skip-cache: true) args: --timeout=5m diff --git a/cmd/thv-operator/controllers/mcpgroup_controller.go b/cmd/thv-operator/controllers/mcpgroup_controller.go index 0c86821500..17e81aac4a 100644 --- a/cmd/thv-operator/controllers/mcpgroup_controller.go +++ b/cmd/thv-operator/controllers/mcpgroup_controller.go @@ -285,6 +285,7 @@ func (r *MCPGroupReconciler) handleDeletion(ctx context.Context, mcpGroup *mcpv1 if err := r.Update(ctx, mcpGroup); err != nil { if errors.IsConflict(err) { // Requeue to retry with fresh data + //nolint:staticcheck // Requeue preserves the controller's rate-limited conflict retry behavior. return ctrl.Result{Requeue: true}, nil } ctxLogger.Error(err, "Failed to remove finalizer") diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go index 846747de2d..ca066677c8 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go @@ -557,6 +557,7 @@ func (r *MCPRemoteProxyReconciler) ensureDeployment( ctxLogger.Error(err, "Failed to update Deployment") return ctrl.Result{}, err } + //nolint:staticcheck // Requeue preserves the controller's existing rate-limited update behavior. return ctrl.Result{Requeue: true}, nil } @@ -608,6 +609,7 @@ func (r *MCPRemoteProxyReconciler) ensureService( ctxLogger.Error(err, "Failed to update Service") return ctrl.Result{}, err } + //nolint:staticcheck // Requeue preserves the controller's existing rate-limited update behavior. return ctrl.Result{Requeue: true}, nil } diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_reconciler_test.go b/cmd/thv-operator/controllers/mcpremoteproxy_reconciler_test.go index 226c6ac4f6..a3a981cc0d 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_reconciler_test.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_reconciler_test.go @@ -939,7 +939,8 @@ func TestMCPRemoteProxy_ValidateAuthzPrimaryUpstreamProviderIgnored(t *testing.T authzConfig: &mcpv1beta1.AuthzConfigRef{ Type: mcpv1beta1.AuthzConfigTypeInline, Inline: &mcpv1beta1.InlineAuthzConfig{ - Policies: []string{`permit(principal, action, resource);`}, + Policies: []string{`permit(principal, action, resource);`}, + //nolint:staticcheck // Exercises backward compatibility for the deprecated field. PrimaryUpstreamProvider: "okta", }, }, diff --git a/cmd/thv-operator/controllers/mcpserver_authz_test.go b/cmd/thv-operator/controllers/mcpserver_authz_test.go index d81e2a28af..d01237a4d3 100644 --- a/cmd/thv-operator/controllers/mcpserver_authz_test.go +++ b/cmd/thv-operator/controllers/mcpserver_authz_test.go @@ -355,7 +355,8 @@ func TestValidateAuthzPrimaryUpstreamProviderIgnored(t *testing.T) { authzConfig: &mcpv1beta1.AuthzConfigRef{ Type: mcpv1beta1.AuthzConfigTypeInline, Inline: &mcpv1beta1.InlineAuthzConfig{ - Policies: []string{`permit(principal, action, resource);`}, + Policies: []string{`permit(principal, action, resource);`}, + //nolint:staticcheck // Exercises the advisory emitted for the deprecated field. PrimaryUpstreamProvider: "okta", }, }, diff --git a/cmd/thv-operator/controllers/mcpserver_controller.go b/cmd/thv-operator/controllers/mcpserver_controller.go index 86e6dbb907..74e643d315 100644 --- a/cmd/thv-operator/controllers/mcpserver_controller.go +++ b/cmd/thv-operator/controllers/mcpserver_controller.go @@ -288,6 +288,7 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } else if shouldTriggerRestart { // Return and requeue to avoid double-processing after triggering restart + //nolint:staticcheck // Requeue preserves the controller's existing rate-limited update behavior. return ctrl.Result{Requeue: true}, nil } @@ -513,6 +514,7 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } // Deployment created successfully - return and requeue + //nolint:staticcheck // Requeue preserves the controller's existing rate-limited update behavior. return ctrl.Result{Requeue: true}, nil } else if err != nil { ctxLogger.Error(err, "Failed to get Deployment") @@ -533,6 +535,7 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } // Spec updated - return and requeue + //nolint:staticcheck // Requeue preserves the controller's existing rate-limited update behavior. return ctrl.Result{Requeue: true}, nil } @@ -554,6 +557,7 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } // Service created successfully - return and requeue + //nolint:staticcheck // Requeue preserves the controller's existing rate-limited create behavior. return ctrl.Result{Requeue: true}, nil } else if err != nil { ctxLogger.Error(err, "Failed to get Service") @@ -612,6 +616,7 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } // Spec updated - return and requeue + //nolint:staticcheck // Requeue preserves the controller's existing rate-limited update behavior. return ctrl.Result{Requeue: true}, nil } @@ -632,6 +637,7 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } // Spec updated - return and requeue + //nolint:staticcheck // Requeue preserves the controller's existing rate-limited update behavior. return ctrl.Result{Requeue: true}, nil } diff --git a/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go b/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go index 31dee190da..00be797179 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go @@ -3530,7 +3530,8 @@ func TestVirtualMCPServerValidateAuthzUpstreamAvailable(t *testing.T) { return &mcpv1beta1.AuthzConfigRef{ Type: "inline", Inline: &mcpv1beta1.InlineAuthzConfig{ - Policies: []string{`permit(principal, action, resource);`}, + Policies: []string{`permit(principal, action, resource);`}, + //nolint:staticcheck // Exercises backward compatibility for the deprecated field. PrimaryUpstreamProvider: primary, }, } @@ -3829,7 +3830,8 @@ func TestVirtualMCPServerValidateAuthzUpstreamAvailable_DeprecationEvent(t *test inlineAuthzRefWithDeprecatedPrimary := &mcpv1beta1.AuthzConfigRef{ Type: "inline", Inline: &mcpv1beta1.InlineAuthzConfig{ - Policies: []string{`permit(principal, action, resource);`}, + Policies: []string{`permit(principal, action, resource);`}, + //nolint:staticcheck // Exercises backward compatibility for the deprecated field. PrimaryUpstreamProvider: "okta", }, } diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go index 575d9d1fe7..617689f928 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go @@ -2127,7 +2127,8 @@ func TestConvertIncomingAuth_PrimaryUpstreamProvider(t *testing.T) { authzConfig: &mcpv1beta1.AuthzConfigRef{ Type: "inline", Inline: &mcpv1beta1.InlineAuthzConfig{ - Policies: []string{`permit(principal, action, resource);`}, + Policies: []string{`permit(principal, action, resource);`}, + //nolint:staticcheck // Exercises conversion of the deprecated field. PrimaryUpstreamProvider: "okta", }, }, @@ -2162,7 +2163,8 @@ func TestConvertIncomingAuth_PrimaryUpstreamProvider(t *testing.T) { authzConfig: &mcpv1beta1.AuthzConfigRef{ Type: "inline", Inline: &mcpv1beta1.InlineAuthzConfig{ - Policies: []string{`permit(principal, action, resource);`}, + Policies: []string{`permit(principal, action, resource);`}, + //nolint:staticcheck // Exercises conversion of the deprecated field. PrimaryUpstreamProvider: "github", }, }, @@ -2182,7 +2184,8 @@ func TestConvertIncomingAuth_PrimaryUpstreamProvider(t *testing.T) { authzConfig: &mcpv1beta1.AuthzConfigRef{ Type: "inline", Inline: &mcpv1beta1.InlineAuthzConfig{ - Policies: []string{`permit(principal, action, resource);`}, + Policies: []string{`permit(principal, action, resource);`}, + //nolint:staticcheck // Exercises conversion of the deprecated field. PrimaryUpstreamProvider: "okta", }, }, diff --git a/cmd/thv/app/llm.go b/cmd/thv/app/llm.go index fb002f799a..13b41341e3 100644 --- a/cmd/thv/app/llm.go +++ b/cmd/thv/app/llm.go @@ -374,7 +374,10 @@ func oidcLogin(ctx context.Context, cfg *llm.Config, skipBrowser bool) error { if err != nil { return fmt.Errorf("building token source: %w", err) } - _, err = ts.Token(ctx) + token, err := ts.Token(ctx) + if err == nil { + cfg.SetDiscoveryAccessToken(token) + } return err } diff --git a/pkg/authserver/spiffe_trust_test.go b/pkg/authserver/spiffe_trust_test.go index a7dfb8dcf4..b933f80877 100644 --- a/pkg/authserver/spiffe_trust_test.go +++ b/pkg/authserver/spiffe_trust_test.go @@ -86,19 +86,19 @@ func TestValidateSPIFFETrust(t *testing.T) { valid := func() ([]SPIFFETrustDomainRunConfig, *InboundGrantsRunConfig) { return []SPIFFETrustDomainRunConfig{{ - Name: "production", - TrustDomain: "example.org", - Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509, SPIFFEAuthenticationMethodJWT}, - BundleSource: validWorkloadAPIBundleSource(), - }}, &InboundGrantsRunConfig{SPIFFEClientAuth: []SPIFFEClientAuthRunConfig{{ - TrustDomainRef: "production", - PrincipalPattern: "spiffe://example.org/ns/default/*", - ClientID: "agent-client", - Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509}, - Audiences: []string{"https://mcp.example.org/resource"}, - Scopes: []string{"openid"}, - GrantTypes: []string{SPIFFEGrantTypeTokenExchange}, - }}} + Name: "production", + TrustDomain: "example.org", + Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509, SPIFFEAuthenticationMethodJWT}, + BundleSource: validWorkloadAPIBundleSource(), + }}, &InboundGrantsRunConfig{SPIFFEClientAuth: []SPIFFEClientAuthRunConfig{{ + TrustDomainRef: "production", + PrincipalPattern: "spiffe://example.org/ns/default/*", + ClientID: "agent-client", + Methods: []SPIFFEAuthenticationMethod{SPIFFEAuthenticationMethodX509}, + Audiences: []string{"https://mcp.example.org/resource"}, + Scopes: []string{"openid"}, + GrantTypes: []string{SPIFFEGrantTypeTokenExchange}, + }}} } tests := []struct { diff --git a/pkg/client/config.go b/pkg/client/config.go index 118d34afb2..2505339c29 100644 --- a/pkg/client/config.go +++ b/pkg/client/config.go @@ -425,19 +425,15 @@ var supportedClientIntegrations = []clientAppConfig{ SupportsSkills: true, SkillsGlobalPath: []string{".copilot", skillsDirName}, SkillsProjectPath: []string{".github", skillsDirName}, - // LLM gateway: patches settings.json (same dir as mcp.json, different file) - LLMGatewayMode: llmgateway.ModeProxy, - LLMSettingsFile: "settings.json", + // VS Code reads custom LLM provider groups from chatLanguageModels.json. + LLMGatewayMode: llmgateway.ModeVSCode, + LLMSettingsFile: "chatLanguageModels.json", LLMSettingsRelPath: []string{"Code - Insiders", "User"}, LLMSettingsPlatformPrefix: map[Platform][]string{ PlatformLinux: {".config"}, PlatformDarwin: {"Library", "Application Support"}, PlatformWindows: {"AppData", "Roaming"}, }, - LLMGatewayKeys: []LLMGatewayKeySpec{ - {JSONPointer: "/github.copilot.advanced.serverUrl", ValueField: "ProxyBaseURL"}, - {JSONPointer: "/github.copilot.advanced.apiKey", ValueField: "PlaceholderAPIKey"}, - }, }, { ClientType: VSCode, @@ -467,19 +463,15 @@ var supportedClientIntegrations = []clientAppConfig{ SupportsSkills: true, SkillsGlobalPath: []string{".copilot", skillsDirName}, SkillsProjectPath: []string{".github", skillsDirName}, - // LLM gateway: patches settings.json (same dir as mcp.json, different file) - LLMGatewayMode: llmgateway.ModeProxy, - LLMSettingsFile: "settings.json", + // VS Code reads custom LLM provider groups from chatLanguageModels.json. + LLMGatewayMode: llmgateway.ModeVSCode, + LLMSettingsFile: "chatLanguageModels.json", LLMSettingsRelPath: []string{"Code", "User"}, LLMSettingsPlatformPrefix: map[Platform][]string{ PlatformLinux: {".config"}, PlatformDarwin: {"Library", "Application Support"}, PlatformWindows: {"AppData", "Roaming"}, }, - LLMGatewayKeys: []LLMGatewayKeySpec{ - {JSONPointer: "/github.copilot.advanced.serverUrl", ValueField: "ProxyBaseURL"}, - {JSONPointer: "/github.copilot.advanced.apiKey", ValueField: "PlaceholderAPIKey"}, - }, }, { ClientType: Cursor, diff --git a/pkg/client/llm_gateway.go b/pkg/client/llm_gateway.go index 7bf3859bf0..a5cdd093fd 100644 --- a/pkg/client/llm_gateway.go +++ b/pkg/client/llm_gateway.go @@ -52,6 +52,9 @@ func (cm *ClientManager) ConfigureLLMGateway(clientType ClientApp, cfg llmgatewa if appCfg.LLMGatewayMode == llmgateway.ModeCodexAuth { return cm.configureCodexAuth(appCfg, cfg) } + if appCfg.LLMGatewayMode == llmgateway.ModeVSCode { + return cm.configureVSCode(appCfg, cfg) + } path := cm.buildLLMSettingsPath(appCfg) @@ -188,6 +191,9 @@ func (cm *ClientManager) RevertLLMGateway(clientType ClientApp, configPath strin if appCfg.LLMGatewayMode == llmgateway.ModeCodexAuth { return cm.revertCodexAuth(appCfg, configPath) } + if appCfg.LLMGatewayMode == llmgateway.ModeVSCode { + return cm.revertVSCode(appCfg, configPath) + } return revertJSONPointerGateway(appCfg, configPath) } diff --git a/pkg/client/llm_gateway_test.go b/pkg/client/llm_gateway_test.go index d74affeccf..9f7b0b08f4 100644 --- a/pkg/client/llm_gateway_test.go +++ b/pkg/client/llm_gateway_test.go @@ -106,22 +106,6 @@ func TestRealClientConfigs_ConfigureAndRevert(t *testing.T) { "/cursor.general.openAIAPIKey": "thv-proxy", }, }, - { - // ~/./Code/User/settings.json - clientType: VSCode, - wantPointers: map[string]string{ - "/github.copilot.advanced.serverUrl": "http://localhost:14000/v1", - "/github.copilot.advanced.apiKey": "thv-proxy", - }, - }, - { - // ~/./Code - Insiders/User/settings.json - clientType: VSCodeInsider, - wantPointers: map[string]string{ - "/github.copilot.advanced.serverUrl": "http://localhost:14000/v1", - "/github.copilot.advanced.apiKey": "thv-proxy", - }, - }, { // ~/Library/Application Support/GitHub Copilot for Xcode/editorSettings.json clientType: ClientApp(Xcode), @@ -169,6 +153,186 @@ func TestRealClientConfigs_ConfigureAndRevert(t *testing.T) { } } +func TestConfigureLLMGateway_VSCodeCustomEndpoint(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + clientType ClientApp + pathPart string + }{ + {name: "stable", clientType: VSCode, pathPart: "Code"}, + {name: "insiders", clientType: VSCodeInsider, pathPart: "Code - Insiders"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + home := t.TempDir() + cm := NewTestClientManager(home, nil, supportedClientIntegrations, nil) + appCfg := cm.lookupClientAppConfig(tt.clientType) + require.NotNil(t, appCfg) + assert.Equal(t, llmgateway.ModeVSCode, appCfg.LLMGatewayMode) + assert.Equal(t, "chatLanguageModels.json", appCfg.LLMSettingsFile) + assert.Contains(t, appCfg.LLMSettingsRelPath, tt.pathPart) + + path, err := cm.ConfigureLLMGateway(tt.clientType, llmgateway.ApplyConfig{ + ProxyBaseURL: "http://localhost:14000/v1", + DiscoveredModels: []string{"model-a", "model-b"}, + }) + require.NoError(t, err) + assert.Equal(t, "chatLanguageModels.json", filepath.Base(path)) + + var groups []vsCodeProviderGroup + data, err := os.ReadFile(path) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(data, &groups)) + require.Len(t, groups, 1) + assert.Equal(t, "ToolHive", groups[0].Name) + assert.Equal(t, "customendpoint", groups[0].Vendor) + require.Len(t, groups[0].Models, 2) + for i, modelID := range []string{"model-a", "model-b"} { + model := groups[0].Models[i] + assert.Equal(t, modelID, model.ID) + assert.Equal(t, modelID, model.Name) + assert.Equal(t, "http://localhost:14000/v1", model.URL) + assert.True(t, model.ToolCalling) + assert.False(t, model.Vision) + assert.Positive(t, model.MaxInputTokens) + assert.Positive(t, model.MaxOutputTokens) + assert.Equal(t, "Bearer thv-proxy", model.RequestHeaders["Authorization"]) + } + }) + } +} + +func TestConfigureLLMGateway_VSCodePreservesContentAndIsIdempotent(t *testing.T) { + t.Parallel() + + home := t.TempDir() + cm := NewTestClientManager(home, nil, supportedClientIntegrations, nil) + appCfg := cm.lookupClientAppConfig(VSCode) + chatPath := cm.buildLLMSettingsPath(appCfg) + require.NoError(t, os.MkdirAll(filepath.Dir(chatPath), 0o700)) + require.NoError(t, os.WriteFile(chatPath, []byte(`[ + // Keep this provider and its comment. + {"name":"Other","vendor":"other","models":[],"unrelated":true}, + {"name":"ToolHive","vendor":"customendpoint","models":[{"id":"stale"}]} + ]`), 0o600)) + settingsPath := filepath.Join(filepath.Dir(chatPath), "settings.json") + require.NoError(t, os.WriteFile(settingsPath, []byte(`{ + "editor.fontSize": 15, + "github.copilot.enable": {"*": true}, + "github.copilot.advanced.serverUrl": "http://obsolete", + "github.copilot.advanced.apiKey": "obsolete" + }`), 0o600)) + + applyCfg := llmgateway.ApplyConfig{ + ProxyBaseURL: "http://localhost:14000/v1", + DiscoveredModels: []string{"model-a"}, + } + for range 2 { + _, err := cm.ConfigureLLMGateway(VSCode, applyCfg) + require.NoError(t, err) + } + + data, err := os.ReadFile(chatPath) + require.NoError(t, err) + assert.Contains(t, string(data), "// Keep this provider and its comment.") + var groups []map[string]any + standardized, err := hujson.Standardize(data) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(standardized, &groups)) + require.Len(t, groups, 2, "repeat setup must replace, not duplicate, the ToolHive group") + assert.Equal(t, "Other", groups[0]["name"]) + assert.Equal(t, true, groups[0]["unrelated"]) + assert.Equal(t, "ToolHive", groups[1]["name"]) + + settings, err := os.ReadFile(settingsPath) + require.NoError(t, err) + assert.Contains(t, string(settings), `"editor.fontSize"`) + assert.Contains(t, string(settings), `"github.copilot.enable"`) + assert.NotContains(t, string(settings), "github.copilot.advanced.serverUrl") + assert.NotContains(t, string(settings), "github.copilot.advanced.apiKey") +} + +func TestRevertLLMGateway_VSCodeMigratesLegacyConfigPath(t *testing.T) { + t.Parallel() + + home := t.TempDir() + cm := NewTestClientManager(home, nil, supportedClientIntegrations, nil) + legacyPath := filepath.Join(filepath.Dir(cm.buildLLMSettingsPath(cm.lookupClientAppConfig(VSCode))), "settings.json") + require.NoError(t, os.MkdirAll(filepath.Dir(legacyPath), 0o700)) + require.NoError(t, os.WriteFile(legacyPath, []byte(`{ + "editor.fontSize": 15, + "github.copilot.advanced.serverUrl": "http://localhost:14000/v1", + "github.copilot.advanced.apiKey": "thv-proxy" + }`), 0o600)) + + require.NoError(t, cm.RevertLLMGateway(VSCode, legacyPath)) + data, err := os.ReadFile(legacyPath) + require.NoError(t, err) + assert.Contains(t, string(data), `"editor.fontSize"`) + assert.NotContains(t, string(data), "github.copilot.advanced") +} + +func TestConfigureLLMGateway_VSCodeErrors(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + content string + models []string + wantErr string + }{ + {name: "no discovered models", models: nil, wantErr: "returned no models"}, + {name: "malformed JSON", content: `[`, models: []string{"model-a"}, wantErr: "parsing"}, + {name: "wrong root type", content: `{}`, models: []string{"model-a"}, wantErr: "expected an array"}, + {name: "malformed provider", content: `[42]`, models: []string{"model-a"}, wantErr: "provider group"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + home := t.TempDir() + cm := NewTestClientManager(home, nil, supportedClientIntegrations, nil) + path := cm.buildLLMSettingsPath(cm.lookupClientAppConfig(VSCode)) + if tt.content != "" { + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o700)) + require.NoError(t, os.WriteFile(path, []byte(tt.content), 0o600)) + } + _, err := cm.ConfigureLLMGateway(VSCode, llmgateway.ApplyConfig{ + ProxyBaseURL: "http://localhost:14000/v1", DiscoveredModels: tt.models, + }) + require.ErrorContains(t, err, tt.wantErr) + }) + } +} + +func TestRevertLLMGateway_VSCodePreservesUnrelatedGroups(t *testing.T) { + t.Parallel() + + home := t.TempDir() + cm := NewTestClientManager(home, nil, supportedClientIntegrations, nil) + path, err := cm.ConfigureLLMGateway(VSCodeInsider, llmgateway.ApplyConfig{ + ProxyBaseURL: "http://localhost:14000/v1", DiscoveredModels: []string{"model-a"}, + }) + require.NoError(t, err) + data := []byte(`[ + {"name":"Other","vendor":"other","models":[]}, + {"name":"ToolHive","vendor":"customendpoint","models":[]} + ]`) + require.NoError(t, os.WriteFile(path, data, 0o600)) + + require.NoError(t, cm.RevertLLMGateway(VSCodeInsider, path)) + result, err := os.ReadFile(path) + require.NoError(t, err) + var groups []vsCodeProviderGroup + require.NoError(t, json.Unmarshal(result, &groups)) + require.Len(t, groups, 1) + assert.Equal(t, "Other", groups[0].Name) + assert.Equal(t, "other", groups[0].Vendor) +} + // ── helpers ─────────────────────────────────────────────────────────────────── // TestConfigureLLMGateway_ClaudeCodeBedrock verifies that BedrockCompat writes diff --git a/pkg/client/llm_gateway_vscode.go b/pkg/client/llm_gateway_vscode.go new file mode 100644 index 0000000000..a1bec44c25 --- /dev/null +++ b/pkg/client/llm_gateway_vscode.go @@ -0,0 +1,190 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "encoding/json" + "fmt" + "log/slog" + "os" + "path/filepath" + + "github.com/tailscale/hujson" + + "github.com/stacklok/toolhive/pkg/fileutils" + "github.com/stacklok/toolhive/pkg/llmgateway" +) + +const ( + vsCodeProviderName = "ToolHive" + vsCodeProviderVendor = "customendpoint" + // The OpenAI-compatible /v1/models response exposes IDs but not token limits. + // VS Code requires positive values, so ToolHive supplies conservative defaults. + vsCodeMaxInputTokens = 128000 + vsCodeMaxOutputTokens = 16000 +) + +type vsCodeProviderGroup struct { + Name string `json:"name"` + Vendor string `json:"vendor"` + Models []vsCodeModel `json:"models"` +} + +type vsCodeModel struct { + ID string `json:"id"` + Name string `json:"name"` + URL string `json:"url"` + ToolCalling bool `json:"toolCalling"` + Vision bool `json:"vision"` + MaxInputTokens int `json:"maxInputTokens"` + MaxOutputTokens int `json:"maxOutputTokens"` + RequestHeaders map[string]string `json:"requestHeaders"` +} + +func (cm *ClientManager) configureVSCode(appCfg *clientAppConfig, cfg llmgateway.ApplyConfig) (string, error) { + if len(cfg.DiscoveredModels) == 0 { + return "", fmt.Errorf("configuring %s: gateway model discovery returned no models", appCfg.ClientType) + } + path := cm.buildLLMSettingsPath(appCfg) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return "", fmt.Errorf("creating directory for %s: %w", path, err) + } + + models := make([]vsCodeModel, 0, len(cfg.DiscoveredModels)) + for _, id := range cfg.DiscoveredModels { + models = append(models, vsCodeModel{ + ID: id, Name: id, URL: cfg.ProxyBaseURL, + ToolCalling: true, Vision: false, + MaxInputTokens: vsCodeMaxInputTokens, MaxOutputTokens: vsCodeMaxOutputTokens, + RequestHeaders: map[string]string{"Authorization": "Bearer " + llmPlaceholderAPIKey}, + }) + } + group, err := json.Marshal(vsCodeProviderGroup{ + Name: vsCodeProviderName, Vendor: vsCodeProviderVendor, Models: models, + }) + if err != nil { + return "", fmt.Errorf("marshaling VS Code provider group: %w", err) + } + + err = fileutils.WithFileLock(path, func() error { + content, err := readOrInitFile(path, []byte("[]")) + if err != nil { + return err + } + v, err := hujson.Parse(content) + if err != nil { + return fmt.Errorf("parsing %s: %w", path, err) + } + if err := patchVSCodeProviderGroups(&v, path, group); err != nil { + return err + } + formatted, err := hujson.Format(v.Pack()) + if err != nil { + return fmt.Errorf("formatting %s: %w", path, err) + } + return fileutils.AtomicWriteFile(path, formatted, 0o600) + }) + if err != nil { + return "", err + } + + // Cleanup is best-effort after the authoritative new configuration is + // durable. A failure must not make setup lose the path needed by teardown. + if err := removeLegacyVSCodeSettings(filepath.Join(filepath.Dir(path), "settings.json")); err != nil { + slog.Warn("Could not remove obsolete VS Code Copilot settings", "error", err) + } + return path, nil +} + +func (*ClientManager) revertVSCode(_ *clientAppConfig, configPath string) error { + legacyPath := filepath.Join(filepath.Dir(configPath), "settings.json") + if filepath.Base(configPath) == "settings.json" { + legacyPath = configPath + } + if err := removeLegacyVSCodeSettings(legacyPath); err != nil { + return err + } + // Older ToolHive versions persisted settings.json as ConfigPath. That file + // is an object and cannot contain a provider group from the new integration. + if filepath.Base(configPath) == "settings.json" { + return nil + } + if _, err := os.Stat(configPath); os.IsNotExist(err) { + return nil + } + return fileutils.WithFileLock(configPath, func() error { + content, err := os.ReadFile(configPath) // #nosec G304 -- persisted configuration path + if err != nil { + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("reading %s: %w", configPath, err) + } + v, err := hujson.Parse(content) + if err != nil { + return fmt.Errorf("parsing %s: %w", configPath, err) + } + if err := patchVSCodeProviderGroups(&v, configPath, nil); err != nil { + return err + } + formatted, err := hujson.Format(v.Pack()) + if err != nil { + return fmt.Errorf("formatting %s: %w", configPath, err) + } + return fileutils.AtomicWriteFile(configPath, formatted, 0o600) + }) +} + +// patchVSCodeProviderGroups removes all ToolHive-owned provider groups and, +// when replacement is non-nil, appends the replacement. Patching the hujson +// syntax tree preserves comments in unrelated groups. +func patchVSCodeProviderGroups(v *hujson.Value, path string, replacement json.RawMessage) error { + standardized, err := hujson.Standardize(v.Pack()) + if err != nil { + return fmt.Errorf("standardizing %s: %w", path, err) + } + var groups []json.RawMessage + if err := json.Unmarshal(standardized, &groups); err != nil { + return fmt.Errorf("parsing %s: expected an array of provider groups: %w", path, err) + } + + operations := make([]llmPatchOp, 0, len(groups)+1) + for i := len(groups) - 1; i >= 0; i-- { + var identity struct { + Name string `json:"name"` + Vendor string `json:"vendor"` + } + if err := json.Unmarshal(groups[i], &identity); err != nil { + return fmt.Errorf("parsing provider group in %s: %w", path, err) + } + if identity.Name == vsCodeProviderName && identity.Vendor == vsCodeProviderVendor { + operations = append(operations, llmPatchOp{Op: "remove", Path: fmt.Sprintf("/%d", i)}) + } + } + if replacement != nil { + operations = append(operations, llmPatchOp{Op: "add", Path: "/-", Value: replacement}) + } + if len(operations) == 0 { + return nil + } + patch, err := json.Marshal(operations) + if err != nil { + return fmt.Errorf("marshaling provider-group patch for %s: %w", path, err) + } + if err := v.Patch(patch); err != nil { + return fmt.Errorf("patching provider groups in %s: %w", path, err) + } + return nil +} + +func removeLegacyVSCodeSettings(path string) error { + legacy := &clientAppConfig{LLMGatewayKeys: []LLMGatewayKeySpec{ + {JSONPointer: "/github.copilot.advanced.serverUrl"}, + {JSONPointer: "/github.copilot.advanced.apiKey"}, + }} + if err := revertJSONPointerGateway(legacy, path); err != nil { + return fmt.Errorf("removing obsolete VS Code Copilot settings: %w", err) + } + return nil +} diff --git a/pkg/llm/config.go b/pkg/llm/config.go index 8be3928bfc..fd3631c2d6 100644 --- a/pkg/llm/config.go +++ b/pkg/llm/config.go @@ -38,8 +38,15 @@ type Config struct { // consumers consistent on a later plain "thv llm setup". Models []string `yaml:"models,omitempty" json:"models,omitempty"` ConfiguredTools []ToolConfig `yaml:"configured_tools,omitempty" json:"configured_tools,omitempty"` + // discoveryAccessToken is populated only for the duration of setup. It is + // deliberately unexported so authentication material can never be persisted. + discoveryAccessToken string } +// SetDiscoveryAccessToken makes the token obtained by setup's authentication +// flow available to authenticated model discovery without persisting it. +func (c *Config) SetDiscoveryAccessToken(token string) { c.discoveryAccessToken = token } + // BedrockConfig holds settings for configuring Claude Code to reach an LLM // gateway that forwards to AWS Bedrock. It is persisted so that a later plain // "thv llm setup" re-applies these settings rather than silently clearing them. @@ -68,7 +75,7 @@ type ToolConfig struct { // Tool is the canonical tool identifier (e.g. "claude-code", "cursor"). Tool string `yaml:"tool" json:"tool"` // Mode is the authentication mode: one of the llmgateway.Mode* values - // ("direct", "proxy", "credential-helper", "codex-auth"). + // ("direct", "proxy", "credential-helper", "codex-auth", "vscode"). Mode string `yaml:"mode" json:"mode"` // ConfigPath is the absolute path to the tool's config file that was patched. ConfigPath string `yaml:"config_path" json:"config_path"` diff --git a/pkg/llm/setup.go b/pkg/llm/setup.go index 20150c98c6..563bc7a2b2 100644 --- a/pkg/llm/setup.go +++ b/pkg/llm/setup.go @@ -6,12 +6,14 @@ package llm import ( "context" "crypto/tls" + "encoding/json" "errors" "fmt" "io" "net/http" "net/url" "os" + "sort" "strings" "time" @@ -79,20 +81,9 @@ func Setup( inlineOpts SetOptions, anthropicPathPrefix string, anthropicPathPrefixSet bool, targetClient string, lazy bool, ) error { - llmCfg := provider.GetLLMConfig() - - // Apply inline flags in-memory so login and tool detection use the merged - // config without touching disk. Persistence happens below, only after login - // and tool patching succeed, so a failed login leaves no persisted state. - if err := llmCfg.SetFields(inlineOpts); err != nil { - if portErr := setupCallbackPortError(err); portErr != nil { - return fmt.Errorf("invalid inline flag values: %w", portErr) - } - return fmt.Errorf("invalid inline flag values: %w", err) - } - - if !llmCfg.IsConfigured() { - return fmt.Errorf("LLM gateway is not configured — run \"thv llm config set\" first") + llmCfg, err := prepareSetupConfig(provider, inlineOpts) + if err != nil { + return err } proxyBaseURL := fmt.Sprintf("http://localhost:%d/v1", llmCfg.EffectiveProxyPort()) @@ -101,12 +92,11 @@ func Setup( // there is nothing to configure. In non-lazy mode login still runs before any // files are patched, preserving the guarantee that a failed login leaves no // state. - detected, err := filterDetectedClients(gm, targetClient) + detected, done, err := setupClients(gm, targetClient, lazy, out, errOut) if err != nil { return err } - if len(detected) == 0 { - _, _ = fmt.Fprintln(out, "No supported AI tools detected.") + if done { return nil } @@ -134,6 +124,16 @@ func Setup( _, _ = fmt.Fprintln(out, "Login successful.") } + detected, discoveredModels, err := discoverVSCodeModelsForSetup( + ctx, errOut, detected, targetClient, llmCfg, + ) + if err != nil { + return err + } + if len(detected) == 0 { + return nil + } + // Resolve the effective path prefix for ANTHROPIC_BASE_URL. // If the caller supplied --anthropic-path-prefix, use it directly. // Otherwise auto-probe: a HEAD request to /anthropic/v1/messages @@ -143,9 +143,9 @@ func Setup( // tools ignore the Anthropic prefix entirely. anthropicPrefix := resolveAnthropicPrefix(ctx, gm, detected, llmCfg, anthropicPathPrefix, anthropicPathPrefixSet) - configured, err := configureDetectedTools( + configured, err := configureDetectedToolsWithDiscovery( out, errOut, gm, detected, llmCfg.GatewayURL, proxyBaseURL, - tokenHelperPath, tokenHelperArgs, llmCfg.TLSSkipVerify, anthropicPrefix, llmCfg.Models, llmCfg.Bedrock, + tokenHelperPath, tokenHelperArgs, llmCfg.TLSSkipVerify, anthropicPrefix, llmCfg.Models, discoveredModels, llmCfg.Bedrock, ) if err != nil { return err @@ -186,6 +186,23 @@ func Setup( return nil } +func prepareSetupConfig(provider ConfigUpdater, inlineOpts SetOptions) (Config, error) { + llmCfg := provider.GetLLMConfig() + // Apply inline flags in-memory so login and tool detection use the merged + // config without touching disk. Persistence happens only after login and + // tool patching succeed, so a failed login leaves no persisted state. + if err := llmCfg.SetFields(inlineOpts); err != nil { + if portErr := setupCallbackPortError(err); portErr != nil { + return Config{}, fmt.Errorf("invalid inline flag values: %w", portErr) + } + return Config{}, fmt.Errorf("invalid inline flag values: %w", err) + } + if !llmCfg.IsConfigured() { + return Config{}, fmt.Errorf("LLM gateway is not configured — run \"thv llm config set\" first") + } + return llmCfg, nil +} + func setupCallbackPortError(err error) error { var portErr *networking.CallbackPortInUseError if !errors.As(err, &portErr) { @@ -364,7 +381,7 @@ func mergeToolConfigs(existing, incoming []ToolConfig) []ToolConfig { // - direct (Node.js tools like Claude Code, Gemini CLI): NODE_TLS_REJECT_UNAUTHORIZED=0 // is written to the tool's settings, disabling TLS for ALL of that tool's outbound // connections — not just the LLM gateway. -// - proxy: only the proxy's upstream connection to the gateway has TLS verification +// - proxy-backed: only the proxy's upstream connection to the gateway has TLS verification // disabled; the tool itself is unaffected. func warnTLSSkipVerify(errOut io.Writer, skip bool, configured []ToolConfig) { if !skip { @@ -378,7 +395,7 @@ func warnTLSSkipVerify(errOut io.Writer, skip bool, configured []ToolConfig) { "settings, disabling TLS certificate verification for ALL of %s's outbound connections "+ "(LLM provider APIs, MCP registry, etc.), not just the LLM gateway. "+ "Use only in isolated local environments.\n", tc.Tool, tc.Tool) - case llmgateway.ModeProxy: + case llmgateway.ModeProxy, llmgateway.ModeVSCode: if tc.Tool == "gemini-cli" { _, _ = fmt.Fprintf(errOut, "Note: --tls-skip-verify is not supported for Gemini CLI "+ @@ -417,6 +434,152 @@ func filterDetectedClients(gm GatewayManager, targetClient string) ([]string, er return nil, fmt.Errorf("client %q is not installed or not detected", targetClient) } +func setupClients( + gm GatewayManager, targetClient string, lazy bool, out, errOut io.Writer, +) ([]string, bool, error) { + detected, err := filterDetectedClients(gm, targetClient) + if err != nil { + return nil, false, err + } + if len(detected) == 0 { + _, _ = fmt.Fprintln(out, "No supported AI tools detected.") + return nil, true, nil + } + if !lazy { + return detected, false, nil + } + detected, err = filterLazyVSCodeClients(detected, targetClient, errOut) + if err != nil { + return nil, false, err + } + return detected, len(detected) == 0, nil +} + +const ( + vsCodeClient = "vscode" + vsCodeInsiderClient = "vscode-insider" +) + +func isVSCodeClient(clientType string) bool { + return clientType == vsCodeClient || clientType == vsCodeInsiderClient +} + +func hasVSCodeClient(clients []string) bool { + for _, clientType := range clients { + if isVSCodeClient(clientType) { + return true + } + } + return false +} + +func filterLazyVSCodeClients(clients []string, targetClient string, errOut io.Writer) ([]string, error) { + if targetClient != "" && isVSCodeClient(targetClient) { + return nil, fmt.Errorf( + "client %q does not support --lazy: VS Code requires non-lazy setup for authenticated model discovery; "+ + "rerun without --lazy", targetClient) + } + filtered := make([]string, 0, len(clients)) + for _, clientType := range clients { + if isVSCodeClient(clientType) { + _, _ = fmt.Fprintf( + errOut, + "Warning: skipping %s in lazy mode; VS Code requires non-lazy setup for authenticated model discovery.\n", + clientType, + ) + continue + } + filtered = append(filtered, clientType) + } + return filtered, nil +} + +func discoverVSCodeModelsForSetup( + ctx context.Context, errOut io.Writer, clients []string, targetClient string, cfg Config, +) ([]string, []string, error) { + if !hasVSCodeClient(clients) { + return clients, nil, nil + } + models, err := discoverGatewayModels(ctx, cfg) + if err == nil { + return clients, models, nil + } + if isVSCodeClient(targetClient) { + return nil, nil, err + } + + _, _ = fmt.Fprintf( + errOut, + "Warning: skipping auto-detected VS Code clients because gateway model discovery failed: %v\n", + err, + ) + filtered := make([]string, 0, len(clients)) + for _, clientType := range clients { + if !isVSCodeClient(clientType) { + filtered = append(filtered, clientType) + } + } + return filtered, nil, nil +} + +type gatewayModelsResponse struct { + Object string `json:"object"` + Data []struct { + ID string `json:"id"` + } `json:"data"` +} + +func discoverGatewayModels(ctx context.Context, cfg Config) ([]string, error) { + if cfg.discoveryAccessToken == "" { + return nil, fmt.Errorf("discovering gateway models: authenticated setup did not provide an access token") + } + modelsURL, err := url.JoinPath(cfg.GatewayURL, "v1/models") + if err != nil { + return nil, fmt.Errorf("building gateway models URL: %w", err) + } + requestCtx, cancel := context.WithTimeout(ctx, 15*time.Second) + defer cancel() + req, err := http.NewRequestWithContext(requestCtx, http.MethodGet, modelsURL, nil) + if err != nil { + return nil, fmt.Errorf("creating gateway model-discovery request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+cfg.discoveryAccessToken) + resp, err := gatewayHTTPClient(cfg.TLSSkipVerify).Do(req) + if err != nil { + return nil, fmt.Errorf("discovering gateway models: %w", err) + } + defer func() { + _ = resp.Body.Close() + }() + if resp.StatusCode != http.StatusOK { + _, _ = io.Copy(io.Discard, resp.Body) + return nil, fmt.Errorf("discovering gateway models: gateway returned %s", resp.Status) + } + var payload gatewayModelsResponse + if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil { + _, _ = io.Copy(io.Discard, resp.Body) + return nil, fmt.Errorf("decoding gateway model response: %w", err) + } + if payload.Object != "list" || len(payload.Data) == 0 { + return nil, fmt.Errorf("decoding gateway model response: expected a non-empty OpenAI model list") + } + seen := make(map[string]struct{}, len(payload.Data)) + models := make([]string, 0, len(payload.Data)) + for _, model := range payload.Data { + modelID := strings.TrimSpace(model.ID) + if modelID == "" { + return nil, fmt.Errorf("decoding gateway model response: model ID must not be empty") + } + if _, exists := seen[modelID]; exists { + continue + } + seen[modelID] = struct{}{} + models = append(models, modelID) + } + sort.Strings(models) + return models, nil +} + // claudeCodeClient is the canonical client identifier for Claude Code. Declared // here as a string literal because pkg/llm does not import pkg/client (which // owns the ClientApp constant) to avoid an import cycle. @@ -516,16 +679,10 @@ func warnBedrockNoEffect(errOut io.Writer, opts SetOptions, effectiveCompat bool // configureDetectedTools patches each detected tool's config file and returns // the list of successfully configured tools. An error is returned only when no // tool was configured successfully. -func configureDetectedTools( - out, errOut io.Writer, - gm GatewayManager, - detected []string, - gatewayURL, proxyBaseURL string, - tokenHelperPath string, tokenHelperArgs []string, - tlsSkipVerify bool, - anthropicPathPrefix string, - models []string, - bedrock BedrockConfig, +func configureDetectedToolsWithDiscovery( + out, errOut io.Writer, gm GatewayManager, detected []string, + gatewayURL, proxyBaseURL, tokenHelperPath string, tokenHelperArgs []string, + tlsSkipVerify bool, anthropicPathPrefix string, models, discoveredModels []string, bedrock BedrockConfig, ) ([]ToolConfig, error) { var configured []ToolConfig for _, clientType := range detected { @@ -553,6 +710,7 @@ func configureDetectedTools( TokenHelperArgs: tokenHelperArgs, TLSSkipVerify: tlsSkipVerify, Models: models, + DiscoveredModels: discoveredModels, } // Bedrock-compat applies only to Claude Code: it disables the experimental @@ -627,20 +785,7 @@ func probeAnthropicPrefix(ctx context.Context, gatewayURL string, tlsSkipVerify return "" } - // Build an http.Client that honours --tls-skip-verify so the probe works - // against gateways with self-signed certificates (local dev). Clone - // http.DefaultTransport to preserve all production defaults (timeouts, - // ProxyFromEnvironment, HTTP/2, connection pooling) and only toggle - // InsecureSkipVerify. - //nolint:forcetypeassert // DefaultTransport is always *http.Transport - transport := http.DefaultTransport.(*http.Transport).Clone() - if tlsSkipVerify { - if transport.TLSClientConfig == nil { - transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} - } - transport.TLSClientConfig.InsecureSkipVerify = true //nolint:gosec // G402: intentional for local dev with self-signed certs - } - httpClient := &http.Client{Transport: transport} + httpClient := gatewayHTTPClient(tlsSkipVerify) // Use a short timeout so setup is not significantly slowed by an unreachable gateway. probeCtx, cancel := context.WithTimeout(ctx, 5*time.Second) @@ -662,6 +807,20 @@ func probeAnthropicPrefix(ctx context.Context, gatewayURL string, tlsSkipVerify return "" } +// gatewayHTTPClient clones the default transport so gateway requests retain +// its proxy, timeout, HTTP/2, and connection-pooling behavior. +func gatewayHTTPClient(tlsSkipVerify bool) *http.Client { + //nolint:forcetypeassert // DefaultTransport is always *http.Transport + transport := http.DefaultTransport.(*http.Transport).Clone() + if tlsSkipVerify { + if transport.TLSClientConfig == nil { + transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} + } + transport.TLSClientConfig.InsecureSkipVerify = true //nolint:gosec // G402: intentional for local dev with self-signed certs + } + return &http.Client{Transport: transport} +} + // tokenHelperShellCommand is the shell command written into direct-mode tools' // config as their token helper — e.g. Claude Code's apiKeyHelper, which is run // through a shell (execa with shell:true; see anthropics/claude-code#42593). @@ -763,10 +922,10 @@ func rollbackConfiguredTools(errOut io.Writer, gm GatewayManager, configured []T } } -// hasProxyMode reports whether any of the given tool configs uses proxy mode. +// hasProxyMode reports whether any tool requires the localhost proxy. func hasProxyMode(cfgs []ToolConfig) bool { for _, t := range cfgs { - if t.Mode == llmgateway.ModeProxy { + if t.Mode == llmgateway.ModeProxy || t.Mode == llmgateway.ModeVSCode { return true } } diff --git a/pkg/llm/setup_test.go b/pkg/llm/setup_test.go index 76872db72b..092d8e32e3 100644 --- a/pkg/llm/setup_test.go +++ b/pkg/llm/setup_test.go @@ -184,12 +184,12 @@ func TestConfigureDetectedTools_BedrockClaudeCode(t *testing.T) { gm := &capturingGatewayManager{mode: "direct"} var out, errOut bytes.Buffer - _, err := configureDetectedTools( + _, err := configureDetectedToolsWithDiscovery( &out, &errOut, gm, []string{"claude-code"}, "https://gw.example.com", "http://localhost:14000/v1", "/usr/local/bin/thv", []string{"llm", "token", "--skip-browser"}, - false, "/anthropic", nil, + false, "/anthropic", nil, nil, BedrockConfig{Compat: true, Enable1M: true}, ) require.NoError(t, err) @@ -208,12 +208,12 @@ func TestConfigureDetectedTools_BedrockSkippedForNonClaudeCode(t *testing.T) { gm := &capturingGatewayManager{mode: "proxy"} var out, errOut bytes.Buffer - _, err := configureDetectedTools( + _, err := configureDetectedToolsWithDiscovery( &out, &errOut, gm, []string{"cursor"}, "https://gw.example.com", "http://localhost:14000/v1", "/usr/local/bin/thv", []string{"llm", "token", "--skip-browser"}, - false, "", nil, + false, "", nil, nil, BedrockConfig{Compat: true}, ) require.NoError(t, err) @@ -319,11 +319,13 @@ func (s *stubGatewayManager) RevertLLMGateway(clientType, _ string) error { // stubConfigUpdater is a minimal ConfigUpdater for Teardown tests. type stubConfigUpdater struct { - cfg Config + cfg Config + updateCalls int } func (s *stubConfigUpdater) GetLLMConfig() Config { return s.cfg } func (s *stubConfigUpdater) UpdateLLMConfig(fn func(*Config) error) error { + s.updateCalls++ return fn(&s.cfg) } @@ -500,13 +502,17 @@ func TestTeardown_NoPurge_LeavesTokenRefsIntact(t *testing.T) { // successfully, for Setup-level tests. mode is returned by LLMGatewayModeFor; // use "proxy" to avoid the direct-mode Anthropic-prefix probe. type setupGatewayManager struct { - detected []string - mode string - hint string + detected []string + mode string + hint string + configured []string + applied []llmgateway.ApplyConfig } func (g *setupGatewayManager) DetectedLLMGatewayClients() []string { return g.detected } -func (*setupGatewayManager) ConfigureLLMGateway(_ string, _ llmgateway.ApplyConfig) (string, error) { +func (g *setupGatewayManager) ConfigureLLMGateway(client string, cfg llmgateway.ApplyConfig) (string, error) { + g.configured = append(g.configured, client) + g.applied = append(g.applied, cfg) return "/tmp/settings.json", nil } func (g *setupGatewayManager) LLMGatewayModeFor(_ string) string { return g.mode } @@ -561,6 +567,179 @@ func TestSetup_Lazy_SkipsLoginAndPersistsTools(t *testing.T) { assert.Contains(t, stdout.String(), "first") } +func TestSetup_Lazy_VSCodeSelectionSemantics(t *testing.T) { + t.Parallel() + + t.Run("explicit VS Code client fails before login", func(t *testing.T) { + t.Parallel() + gm := &setupGatewayManager{detected: []string{"vscode"}, mode: llmgateway.ModeVSCode} + provider := configuredSetupProvider() + loginCalled := false + var stdout, stderr bytes.Buffer + err := Setup(context.Background(), &stdout, &stderr, gm, provider, + func(_ context.Context, _ *Config) error { loginCalled = true; return nil }, + SetOptions{}, "", true, "vscode", true) + require.ErrorContains(t, err, "requires non-lazy setup for authenticated model discovery") + assert.False(t, loginCalled) + assert.Empty(t, gm.configured) + }) + + t.Run("multi-client setup warns, skips VS Code, and continues", func(t *testing.T) { + t.Parallel() + gm := &setupGatewayManager{detected: []string{"vscode", "cursor", "vscode-insider"}, mode: llmgateway.ModeProxy} + provider := configuredSetupProvider() + loginCalled := false + var stdout, stderr bytes.Buffer + err := Setup(context.Background(), &stdout, &stderr, gm, provider, + func(_ context.Context, _ *Config) error { loginCalled = true; return nil }, + SetOptions{}, "", true, "", true) + require.NoError(t, err) + assert.False(t, loginCalled) + assert.Equal(t, []string{"cursor"}, gm.configured) + assert.Contains(t, stderr.String(), "skipping vscode in lazy mode") + assert.Contains(t, stderr.String(), "skipping vscode-insider in lazy mode") + require.Len(t, provider.cfg.ConfiguredTools, 1) + assert.Equal(t, "cursor", provider.cfg.ConfiguredTools[0].Tool) + }) +} + +func TestDiscoverGatewayModels(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + status int + body string + setToken bool + want []string + wantErr string + }{ + {name: "valid OpenAI list is trimmed, deduplicated, and sorted", status: http.StatusOK, + body: `{"object":"list","data":[{"id":" z-model "},{"id":"a-model"},{"id":"z-model"},` + + `{"id":" a-model "}]}`, + setToken: true, want: []string{"a-model", "z-model"}}, + {name: "missing authentication", status: http.StatusOK, body: `{"object":"list","data":[{"id":"model"}]}`, + wantErr: "did not provide an access token"}, + {name: "authentication failure", status: http.StatusUnauthorized, body: `{"error":"unauthorized"}`, + setToken: true, wantErr: "401 Unauthorized"}, + {name: "malformed response", status: http.StatusOK, body: `{`, setToken: true, + wantErr: "decoding gateway model response"}, + {name: "wrong response shape", status: http.StatusOK, body: `{"object":"other","data":[{"id":"model"}]}`, + setToken: true, wantErr: "expected a non-empty OpenAI model list"}, + {name: "empty model ID", status: http.StatusOK, body: `{"object":"list","data":[{"id":" "}]}`, + setToken: true, wantErr: "model ID must not be empty"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/v1/models", r.URL.Path) + if tt.setToken { + assert.Equal(t, "Bearer discovery-token", r.Header.Get("Authorization")) + } + w.WriteHeader(tt.status) + _, _ = w.Write([]byte(tt.body)) + })) + t.Cleanup(server.Close) + + cfg := Config{GatewayURL: server.URL} + if tt.setToken { + cfg.SetDiscoveryAccessToken("discovery-token") + } + got, err := discoverGatewayModels(context.Background(), cfg) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestSetup_VSCodeDiscoversModelsAfterLogin(t *testing.T) { + t.Parallel() + + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "Bearer discovery-token", r.Header.Get("Authorization")) + _, _ = w.Write([]byte(`{"object":"list","data":[{"id":"model-b"},{"id":"model-a"}]}`)) + })) + t.Cleanup(server.Close) + gm := &setupGatewayManager{detected: []string{"vscode"}, mode: llmgateway.ModeVSCode} + provider := configuredSetupProvider() + provider.cfg.GatewayURL = server.URL + provider.cfg.TLSSkipVerify = true + var stdout, stderr bytes.Buffer + err := Setup(context.Background(), &stdout, &stderr, gm, provider, + func(_ context.Context, cfg *Config) error { cfg.SetDiscoveryAccessToken("discovery-token"); return nil }, + SetOptions{}, "", true, "vscode", false) + require.NoError(t, err) + require.Len(t, gm.applied, 1) + assert.Equal(t, []string{"model-a", "model-b"}, gm.applied[0].DiscoveredModels) + assert.Equal(t, "http://localhost:14000/v1", gm.applied[0].ProxyBaseURL) +} + +func TestSetup_VSCodeDiscoveryFailureSelectionSemantics(t *testing.T) { + t.Parallel() + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + })) + t.Cleanup(server.Close) + providerForTest := func() *stubConfigUpdater { + provider := configuredSetupProvider() + provider.cfg.GatewayURL = server.URL + provider.cfg.TLSSkipVerify = true + return provider + } + + for _, clientType := range []string{"vscode", "vscode-insider"} { + t.Run("explicit "+clientType+" client fails", func(t *testing.T) { + t.Parallel() + gm := &setupGatewayManager{detected: []string{clientType}, mode: llmgateway.ModeVSCode} + provider := providerForTest() + var stdout, stderr bytes.Buffer + err := Setup(context.Background(), &stdout, &stderr, gm, provider, + func(_ context.Context, cfg *Config) error { cfg.SetDiscoveryAccessToken("token"); return nil }, + SetOptions{}, "", true, clientType, false) + require.ErrorContains(t, err, "discovering gateway models") + require.ErrorContains(t, err, "503 Service Unavailable") + assert.Empty(t, gm.configured) + assert.Zero(t, provider.updateCalls) + }) + } + + t.Run("auto-detected VS Code is skipped while other clients continue", func(t *testing.T) { + t.Parallel() + gm := &setupGatewayManager{detected: []string{"vscode", "cursor", "vscode-insider"}, mode: llmgateway.ModeProxy} + provider := providerForTest() + var stdout, stderr bytes.Buffer + err := Setup(context.Background(), &stdout, &stderr, gm, provider, + func(_ context.Context, cfg *Config) error { cfg.SetDiscoveryAccessToken("token"); return nil }, + SetOptions{}, "", true, "", false) + require.NoError(t, err) + assert.Equal(t, []string{"cursor"}, gm.configured) + assert.Contains(t, stderr.String(), "skipping auto-detected VS Code clients") + assert.Contains(t, stderr.String(), "gateway model discovery failed") + assert.Contains(t, stderr.String(), "503 Service Unavailable") + require.Len(t, provider.cfg.ConfiguredTools, 1) + assert.Equal(t, "cursor", provider.cfg.ConfiguredTools[0].Tool) + }) + + t.Run("auto-detected VS Code alone is skipped without persistence", func(t *testing.T) { + t.Parallel() + gm := &setupGatewayManager{detected: []string{"vscode"}, mode: llmgateway.ModeVSCode} + provider := providerForTest() + var stdout, stderr bytes.Buffer + err := Setup(context.Background(), &stdout, &stderr, gm, provider, + func(_ context.Context, cfg *Config) error { cfg.SetDiscoveryAccessToken("token"); return nil }, + SetOptions{}, "", true, "", false) + require.NoError(t, err) + assert.Empty(t, gm.configured) + assert.Zero(t, provider.updateCalls) + assert.Contains(t, stderr.String(), "skipping auto-detected VS Code clients") + }) +} + func TestFilterDetectedClients_LeftoverDirHint(t *testing.T) { t.Parallel() gm := &setupGatewayManager{ @@ -717,12 +896,12 @@ func TestConfigureDetectedTools_PathPrefixAppendedForDirectMode(t *testing.T) { gm := &capturingGatewayManager{mode: "direct"} var out, errOut bytes.Buffer - _, err := configureDetectedTools( + _, err := configureDetectedToolsWithDiscovery( &out, &errOut, gm, []string{"claude-code"}, "https://gw.example.com", "http://localhost:14000/v1", "/usr/local/bin/thv", []string{"llm", "token", "--skip-browser"}, - false, "/anthropic", nil, + false, "/anthropic", nil, nil, BedrockConfig{}, ) require.NoError(t, err) @@ -739,12 +918,12 @@ func TestConfigureDetectedTools_NoPrefixWhenEmpty(t *testing.T) { gm := &capturingGatewayManager{mode: "direct"} var out, errOut bytes.Buffer - _, err := configureDetectedTools( + _, err := configureDetectedToolsWithDiscovery( &out, &errOut, gm, []string{"claude-code"}, "https://gw.example.com", "http://localhost:14000/v1", "/usr/local/bin/thv", []string{"llm", "token", "--skip-browser"}, - false, "", nil, // no prefix + false, "", nil, nil, // no prefix BedrockConfig{}, ) require.NoError(t, err) @@ -760,12 +939,12 @@ func TestConfigureDetectedTools_PrefixNotAppliedForProxyMode(t *testing.T) { gm := &capturingGatewayManager{mode: "proxy"} var out, errOut bytes.Buffer - _, err := configureDetectedTools( + _, err := configureDetectedToolsWithDiscovery( &out, &errOut, gm, []string{"cursor"}, "https://gw.example.com", "http://localhost:14000/v1", "/usr/local/bin/thv", []string{"llm", "token", "--skip-browser"}, - false, "/anthropic", nil, + false, "/anthropic", nil, nil, BedrockConfig{}, ) require.NoError(t, err) @@ -794,6 +973,17 @@ func TestWarnTLSSkipVerify_CodexWarning(t *testing.T) { assert.Contains(t, out, "was NOT applied to codex") } +func TestVSCodeModeRequiresProxyAndUsesProxyTLSWarning(t *testing.T) { + t.Parallel() + + configured := []ToolConfig{{Tool: "vscode", Mode: llmgateway.ModeVSCode}} + assert.True(t, hasProxyMode(configured)) + + var errOut bytes.Buffer + warnTLSSkipVerify(&errOut, true, configured) + assert.Contains(t, errOut.String(), "proxy's upstream gateway connection only") +} + // ── probeAnthropicPrefix ────────────────────────────────────────────────────── func TestProbeAnthropicPrefix_Returns_Anthropic_On_401(t *testing.T) { diff --git a/pkg/llmgateway/config.go b/pkg/llmgateway/config.go index c3795be729..c88a0ce406 100644 --- a/pkg/llmgateway/config.go +++ b/pkg/llmgateway/config.go @@ -65,6 +65,8 @@ const ( // shell), rather than the JSON-Pointer LLMGatewayKeys mechanism the other // modes share. ModeCodexAuth = "codex-auth" //nolint:gosec // G101: mode identifier, not a credential + // ModeVSCode configures VS Code's chatLanguageModels.json provider groups. + ModeVSCode = "vscode" ) // ProxyOriginOf returns rawURL with its path, query, fragment, and userinfo @@ -108,6 +110,10 @@ type ApplyConfig struct { // a model override (e.g. Claude Desktop's inferenceModels). Empty means the // client falls back to gateway-side model auto-discovery. Models []string + // DiscoveredModels contains model IDs returned by the authenticated gateway + // discovery request. It is used only by integrations that require an explicit + // model catalogue, such as VS Code's customendpoint provider. + DiscoveredModels []string // BedrockCompat and the per-tier Bedrock model IDs configure Claude Code for a // gateway that forwards to AWS Bedrock. When BedrockCompat is true, Claude Code // is configured with CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS=1 (Bedrock rejects diff --git a/test/e2e/cli_llm_all_clients_test.go b/test/e2e/cli_llm_all_clients_test.go index 3f331eba35..8bc8e0bfa2 100644 --- a/test/e2e/cli_llm_all_clients_test.go +++ b/test/e2e/cli_llm_all_clients_test.go @@ -135,44 +135,6 @@ func allClientTestCases() []llmClientTestCase { }, }, }, - { - name: "vscode", - detectionDir: func(tempDir string) string { - return llmSettingsDirFor("vscode", tempDir) - }, - binaryName: "", - settingsPath: func(tempDir string) string { - return filepath.Join(llmSettingsDirFor("vscode", tempDir), "settings.json") - }, - mode: "proxy", - expectedKeys: map[string]func(string, string) string{ - "/github.copilot.advanced.serverUrl": func(_, proxyURL string) string { - return proxyURL - }, - "/github.copilot.advanced.apiKey": func(_, _ string) string { - return clientThvProxy - }, - }, - }, - { - name: "vscode-insider", - detectionDir: func(tempDir string) string { - return llmSettingsDirFor("vscode-insider", tempDir) - }, - binaryName: "", - settingsPath: func(tempDir string) string { - return filepath.Join(llmSettingsDirFor("vscode-insider", tempDir), "settings.json") - }, - mode: "proxy", - expectedKeys: map[string]func(string, string) string{ - "/github.copilot.advanced.serverUrl": func(_, proxyURL string) string { - return proxyURL - }, - "/github.copilot.advanced.apiKey": func(_, _ string) string { - return clientThvProxy - }, - }, - }, { name: "xcode", detectionDir: func(tempDir string) string {