diff --git a/internal/config/load_test.go b/internal/config/load_test.go index b6038e0d0e3fb76e4567b4c5c739eb8da952311e..61054e5ccbc9031702f3d1d778634bf53b625bcb 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -77,7 +77,7 @@ func TestConfig_configureProviders(t *testing.T) { env := env.NewFromMap(map[string]string{ "OPENAI_API_KEY": "test-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) require.Equal(t, 1, cfg.Providers.Len()) @@ -120,7 +120,7 @@ func TestConfig_configureProvidersWithOverride(t *testing.T) { env := env.NewFromMap(map[string]string{ "OPENAI_API_KEY": "test-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) require.Equal(t, 1, cfg.Providers.Len()) @@ -162,7 +162,7 @@ func TestConfig_configureProvidersWithNewProvider(t *testing.T) { env := env.NewFromMap(map[string]string{ "OPENAI_API_KEY": "test-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) // Should be to because of the env variable @@ -198,7 +198,7 @@ func TestConfig_configureProvidersBedrockWithCredentials(t *testing.T) { "AWS_ACCESS_KEY_ID": "test-key-id", "AWS_SECRET_ACCESS_KEY": "test-secret-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) require.Equal(t, cfg.Providers.Len(), 1) @@ -224,7 +224,7 @@ func TestConfig_configureProvidersBedrockWithoutCredentials(t *testing.T) { cfg := &Config{} cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) // Provider should not be configured without credentials @@ -249,7 +249,7 @@ func TestConfig_configureProvidersBedrockWithoutUnsupportedModel(t *testing.T) { "AWS_ACCESS_KEY_ID": "test-key-id", "AWS_SECRET_ACCESS_KEY": "test-secret-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.Error(t, err) } @@ -272,7 +272,7 @@ func TestConfig_configureProvidersVertexAIWithCredentials(t *testing.T) { "VERTEXAI_PROJECT": "test-project", "VERTEXAI_LOCATION": "us-central1", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) require.Equal(t, cfg.Providers.Len(), 1) @@ -304,7 +304,7 @@ func TestConfig_configureProvidersVertexAIWithoutCredentials(t *testing.T) { "GOOGLE_CLOUD_PROJECT": "test-project", "GOOGLE_CLOUD_LOCATION": "us-central1", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) // Provider should not be configured without proper credentials @@ -329,7 +329,7 @@ func TestConfig_configureProvidersVertexAIMissingProject(t *testing.T) { "GOOGLE_GENAI_USE_VERTEXAI": "true", "GOOGLE_CLOUD_LOCATION": "us-central1", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) // Provider should not be configured without project @@ -353,7 +353,7 @@ func TestConfig_configureProvidersSetProviderID(t *testing.T) { env := env.NewFromMap(map[string]string{ "OPENAI_API_KEY": "test-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) require.Equal(t, cfg.Providers.Len(), 1) @@ -544,7 +544,7 @@ func TestConfig_configureProvidersWithDisabledProvider(t *testing.T) { env := env.NewFromMap(map[string]string{ "OPENAI_API_KEY": "test-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -572,7 +572,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{}) require.NoError(t, err) @@ -595,7 +595,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{}) require.NoError(t, err) @@ -617,7 +617,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{}) require.NoError(t, err) @@ -642,7 +642,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{}) require.NoError(t, err) @@ -667,7 +667,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{}) require.NoError(t, err) @@ -695,7 +695,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{}) require.NoError(t, err) @@ -725,7 +725,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{}) require.NoError(t, err) @@ -760,7 +760,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) { env := env.NewFromMap(map[string]string{ "GOOGLE_GENAI_USE_VERTEXAI": "false", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -791,7 +791,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -822,7 +822,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -855,7 +855,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) { env := env.NewFromMap(map[string]string{ "OPENAI_API_KEY": "test-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -889,7 +889,7 @@ func TestConfig_defaultModelSelection(t *testing.T) { cfg := &Config{} cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -925,7 +925,7 @@ func TestConfig_defaultModelSelection(t *testing.T) { cfg := &Config{} cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -955,7 +955,7 @@ func TestConfig_defaultModelSelection(t *testing.T) { cfg := &Config{} cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) _, _, err = cfg.defaultModelSelection(knownProviders) @@ -998,7 +998,7 @@ func TestConfig_defaultModelSelection(t *testing.T) { } cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) large, small, err := cfg.defaultModelSelection(knownProviders) @@ -1042,7 +1042,7 @@ func TestConfig_defaultModelSelection(t *testing.T) { } cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) _, _, err = cfg.defaultModelSelection(knownProviders) @@ -1084,7 +1084,7 @@ func TestConfig_defaultModelSelection(t *testing.T) { } cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) large, small, err := cfg.defaultModelSelection(knownProviders) @@ -1129,7 +1129,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) { env := env.NewFromMap(map[string]string{ "OPENAI_API_KEY": "test-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.ErrorContains(t, err, "no custom providers") @@ -1172,7 +1172,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) { "MY_API_KEY": "test-key", "OPENAI_API_KEY": "test-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -1226,7 +1226,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) { "OPENAI_API_KEY": "test-key", "ANTHROPIC_API_KEY": "test-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -1254,7 +1254,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{}) require.ErrorContains(t, err, "no custom providers") @@ -1278,7 +1278,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) { cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{}) require.ErrorContains(t, err, "no custom providers") @@ -1336,7 +1336,7 @@ func TestConfig_configureSelectedModels(t *testing.T) { cfg.setDefaults(dir, "") store := &ConfigStore{config: cfg, globalDataPath: globalPath} env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(store, env, resolver, knownProviders) require.NoError(t, err) @@ -1386,7 +1386,7 @@ func TestConfig_configureSelectedModels(t *testing.T) { } cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -1448,7 +1448,7 @@ func TestConfig_configureSelectedModels(t *testing.T) { } cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -1493,7 +1493,7 @@ func TestConfig_configureSelectedModels(t *testing.T) { } cfg.setDefaults("/tmp", "") env := env.NewFromMap(map[string]string{}) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) @@ -1532,7 +1532,7 @@ func TestConfig_configureProviders_HyperAPIKeyFromEnv(t *testing.T) { env := env.NewFromMap(map[string]string{ "HYPER_API_KEY": "env-api-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) require.Equal(t, 1, cfg.Providers.Len()) @@ -1579,7 +1579,7 @@ func TestConfig_configureProviders_HyperAPIKeyFromConfigOverrides(t *testing.T) env := env.NewFromMap(map[string]string{ "HYPER_API_KEY": "env-api-key", }) - resolver := NewEnvironmentVariableResolver(env) + resolver := NewShellVariableResolver(env) err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders) require.NoError(t, err) require.Equal(t, 1, cfg.Providers.Len()) diff --git a/internal/config/resolve.go b/internal/config/resolve.go index 8e13e16d440a31b73672ab55a467cc7ff866a1b8..a4a359be2e4ce1f6be32eba99b1b68eb3f61fe25 100644 --- a/internal/config/resolve.go +++ b/internal/config/resolve.go @@ -10,8 +10,7 @@ import ( ) // resolveTimeout bounds how long a single ResolveValue call may spend -// inside shell expansion (including any command substitution). Matches -// the timeout used by the previous hand-rolled parser. +// inside shell expansion (including any command substitution). const resolveTimeout = 5 * time.Minute type VariableResolver interface { @@ -173,27 +172,3 @@ func scrubErrorMessage(s string) string { } return string(out) } - -type environmentVariableResolver struct { - env env.Env -} - -func NewEnvironmentVariableResolver(env env.Env) VariableResolver { - return &environmentVariableResolver{ - env: env, - } -} - -// ResolveValue resolves environment variables from the provided env.Env. -func (r *environmentVariableResolver) ResolveValue(value string) (string, error) { - if len(value) == 0 || value[0] != '$' { - return value, nil - } - - varName := value[1:] - resolvedValue := r.env.Get(varName) - if resolvedValue == "" { - return "", fmt.Errorf("environment variable %q not set", varName) - } - return resolvedValue, nil -} diff --git a/internal/config/resolve_test.go b/internal/config/resolve_test.go index 18b461b8a6bc88f73d6db935eebd302cc3514781..3d364e523ae4738d23888941bc429d2c00241fa2 100644 --- a/internal/config/resolve_test.go +++ b/internal/config/resolve_test.go @@ -220,62 +220,6 @@ func TestScrubErrorMessage(t *testing.T) { }) } -func TestEnvironmentVariableResolver_ResolveValue(t *testing.T) { - tests := []struct { - name string - value string - envVars map[string]string - expected string - expectError bool - }{ - { - name: "non-variable string returns as-is", - value: "plain-string", - expected: "plain-string", - }, - { - name: "environment variable resolution", - value: "$HOME", - envVars: map[string]string{"HOME": "/home/user"}, - expected: "/home/user", - }, - { - name: "environment variable with complex value", - value: "$PATH", - envVars: map[string]string{"PATH": "/usr/bin:/bin:/usr/local/bin"}, - expected: "/usr/bin:/bin:/usr/local/bin", - }, - { - name: "missing environment variable returns error", - value: "$MISSING_VAR", - envVars: map[string]string{}, - expectError: true, - }, - { - name: "empty environment variable returns error", - value: "$EMPTY_VAR", - envVars: map[string]string{"EMPTY_VAR": ""}, - expectError: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - testEnv := env.NewFromMap(tt.envVars) - resolver := NewEnvironmentVariableResolver(testEnv) - - result, err := resolver.ResolveValue(tt.value) - - if tt.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) - require.Equal(t, tt.expected, result) - } - }) - } -} - func TestNewShellVariableResolver(t *testing.T) { testEnv := env.NewFromMap(map[string]string{"TEST": "value"}) resolver := NewShellVariableResolver(testEnv) @@ -283,11 +227,3 @@ func TestNewShellVariableResolver(t *testing.T) { require.NotNil(t, resolver) require.Implements(t, (*VariableResolver)(nil), resolver) } - -func TestNewEnvironmentVariableResolver(t *testing.T) { - testEnv := env.NewFromMap(map[string]string{"TEST": "value"}) - resolver := NewEnvironmentVariableResolver(testEnv) - - require.NotNil(t, resolver) - require.Implements(t, (*VariableResolver)(nil), resolver) -} diff --git a/internal/lsp/client_test.go b/internal/lsp/client_test.go index 0337dc8b9ba506264e519e1c53a725ab9fec35b1..583f7a8962600d711ab5087a7edf99d22eedba6b 100644 --- a/internal/lsp/client_test.go +++ b/internal/lsp/client_test.go @@ -26,7 +26,7 @@ func TestClient(t *testing.T) { // Test creating a powernap client - this will likely fail with echo // but we can still test the basic structure - client, err := New(ctx, "test", cfg, config.NewEnvironmentVariableResolver(env.NewFromMap(map[string]string{ + client, err := New(ctx, "test", cfg, config.NewShellVariableResolver(env.NewFromMap(map[string]string{ "THE_CMD": "echo", })), ".", false) if err != nil {