on main help sign in
config: remove unused environment variable resolver

The hand-rolled $VARNAME-only resolver (NewEnvironmentVariableResolver) had no production callers. Tests have been switched to the shell resolver, which is what production already uses, and the dead type plus its tests are gone. Co-Authored-By: Charm Crush <crush@charm.land>

commitf9134777c941001444eb57ecc81bfcb49c6366f5 authorChristian Rocha <christian@rocha.is> dateSun 3 May 2026, 17:02 (6 months ago) parent06f4350
4 files changed+41−130
internal/config/load_test.go+39−39 internal/config/resolve.go+1−26 internal/config/resolve_test.go−64 internal/lsp/client_test.go+1−1
internal/config/load_test.go+39−39
@@ -77,7 +77,7 @@ func TestConfig_configureProviders(t *testing.T) {
7777 env := env.NewFromMap(map[string]string{
7878 "OPENAI_API_KEY": "test-key",
7979 })
80− resolver := NewEnvironmentVariableResolver(env)
80+ resolver := NewShellVariableResolver(env)
8181 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
8282 require.NoError(t, err)
8383 require.Equal(t, 1, cfg.Providers.Len())
@@ -120,7 +120,7 @@ func TestConfig_configureProvidersWithOverride(t *testing.T) {
120120 env := env.NewFromMap(map[string]string{
121121 "OPENAI_API_KEY": "test-key",
122122 })
123− resolver := NewEnvironmentVariableResolver(env)
123+ resolver := NewShellVariableResolver(env)
124124 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
125125 require.NoError(t, err)
126126 require.Equal(t, 1, cfg.Providers.Len())
@@ -162,7 +162,7 @@ func TestConfig_configureProvidersWithNewProvider(t *testing.T) {
162162 env := env.NewFromMap(map[string]string{
163163 "OPENAI_API_KEY": "test-key",
164164 })
165− resolver := NewEnvironmentVariableResolver(env)
165+ resolver := NewShellVariableResolver(env)
166166 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
167167 require.NoError(t, err)
168168 // Should be to because of the env variable
@@ -198,7 +198,7 @@ func TestConfig_configureProvidersBedrockWithCredentials(t *testing.T) {
198198 "AWS_ACCESS_KEY_ID": "test-key-id",
199199 "AWS_SECRET_ACCESS_KEY": "test-secret-key",
200200 })
201− resolver := NewEnvironmentVariableResolver(env)
201+ resolver := NewShellVariableResolver(env)
202202 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
203203 require.NoError(t, err)
204204 require.Equal(t, cfg.Providers.Len(), 1)
@@ -224,7 +224,7 @@ func TestConfig_configureProvidersBedrockWithoutCredentials(t *testing.T) {
224224 cfg := &Config{}
225225 cfg.setDefaults("/tmp", "")
226226 env := env.NewFromMap(map[string]string{})
227− resolver := NewEnvironmentVariableResolver(env)
227+ resolver := NewShellVariableResolver(env)
228228 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
229229 require.NoError(t, err)
230230 // Provider should not be configured without credentials
@@ -249,7 +249,7 @@ func TestConfig_configureProvidersBedrockWithoutUnsupportedModel(t *testing.T) {
249249 "AWS_ACCESS_KEY_ID": "test-key-id",
250250 "AWS_SECRET_ACCESS_KEY": "test-secret-key",
251251 })
252− resolver := NewEnvironmentVariableResolver(env)
252+ resolver := NewShellVariableResolver(env)
253253 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
254254 require.Error(t, err)
255255}
@@ -272,7 +272,7 @@ func TestConfig_configureProvidersVertexAIWithCredentials(t *testing.T) {
272272 "VERTEXAI_PROJECT": "test-project",
273273 "VERTEXAI_LOCATION": "us-central1",
274274 })
275− resolver := NewEnvironmentVariableResolver(env)
275+ resolver := NewShellVariableResolver(env)
276276 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
277277 require.NoError(t, err)
278278 require.Equal(t, cfg.Providers.Len(), 1)
@@ -304,7 +304,7 @@ func TestConfig_configureProvidersVertexAIWithoutCredentials(t *testing.T) {
304304 "GOOGLE_CLOUD_PROJECT": "test-project",
305305 "GOOGLE_CLOUD_LOCATION": "us-central1",
306306 })
307− resolver := NewEnvironmentVariableResolver(env)
307+ resolver := NewShellVariableResolver(env)
308308 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
309309 require.NoError(t, err)
310310 // Provider should not be configured without proper credentials
@@ -329,7 +329,7 @@ func TestConfig_configureProvidersVertexAIMissingProject(t *testing.T) {
329329 "GOOGLE_GENAI_USE_VERTEXAI": "true",
330330 "GOOGLE_CLOUD_LOCATION": "us-central1",
331331 })
332− resolver := NewEnvironmentVariableResolver(env)
332+ resolver := NewShellVariableResolver(env)
333333 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
334334 require.NoError(t, err)
335335 // Provider should not be configured without project
@@ -353,7 +353,7 @@ func TestConfig_configureProvidersSetProviderID(t *testing.T) {
353353 env := env.NewFromMap(map[string]string{
354354 "OPENAI_API_KEY": "test-key",
355355 })
356− resolver := NewEnvironmentVariableResolver(env)
356+ resolver := NewShellVariableResolver(env)
357357 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
358358 require.NoError(t, err)
359359 require.Equal(t, cfg.Providers.Len(), 1)
@@ -544,7 +544,7 @@ func TestConfig_configureProvidersWithDisabledProvider(t *testing.T) {
544544 env := env.NewFromMap(map[string]string{
545545 "OPENAI_API_KEY": "test-key",
546546 })
547− resolver := NewEnvironmentVariableResolver(env)
547+ resolver := NewShellVariableResolver(env)
548548 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
549549 require.NoError(t, err)
550550
@@ -572,7 +572,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {
572572 cfg.setDefaults("/tmp", "")
573573
574574 env := env.NewFromMap(map[string]string{})
575− resolver := NewEnvironmentVariableResolver(env)
575+ resolver := NewShellVariableResolver(env)
576576 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})
577577 require.NoError(t, err)
578578
@@ -595,7 +595,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {
595595 cfg.setDefaults("/tmp", "")
596596
597597 env := env.NewFromMap(map[string]string{})
598− resolver := NewEnvironmentVariableResolver(env)
598+ resolver := NewShellVariableResolver(env)
599599 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})
600600 require.NoError(t, err)
601601
@@ -617,7 +617,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {
617617 cfg.setDefaults("/tmp", "")
618618
619619 env := env.NewFromMap(map[string]string{})
620− resolver := NewEnvironmentVariableResolver(env)
620+ resolver := NewShellVariableResolver(env)
621621 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})
622622 require.NoError(t, err)
623623
@@ -642,7 +642,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {
642642 cfg.setDefaults("/tmp", "")
643643
644644 env := env.NewFromMap(map[string]string{})
645− resolver := NewEnvironmentVariableResolver(env)
645+ resolver := NewShellVariableResolver(env)
646646 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})
647647 require.NoError(t, err)
648648
@@ -667,7 +667,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {
667667 cfg.setDefaults("/tmp", "")
668668
669669 env := env.NewFromMap(map[string]string{})
670− resolver := NewEnvironmentVariableResolver(env)
670+ resolver := NewShellVariableResolver(env)
671671 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})
672672 require.NoError(t, err)
673673
@@ -695,7 +695,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {
695695 cfg.setDefaults("/tmp", "")
696696
697697 env := env.NewFromMap(map[string]string{})
698− resolver := NewEnvironmentVariableResolver(env)
698+ resolver := NewShellVariableResolver(env)
699699 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})
700700 require.NoError(t, err)
701701
@@ -725,7 +725,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {
725725 cfg.setDefaults("/tmp", "")
726726
727727 env := env.NewFromMap(map[string]string{})
728− resolver := NewEnvironmentVariableResolver(env)
728+ resolver := NewShellVariableResolver(env)
729729 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})
730730 require.NoError(t, err)
731731
@@ -760,7 +760,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) {
760760 env := env.NewFromMap(map[string]string{
761761 "GOOGLE_GENAI_USE_VERTEXAI": "false",
762762 })
763− resolver := NewEnvironmentVariableResolver(env)
763+ resolver := NewShellVariableResolver(env)
764764 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
765765 require.NoError(t, err)
766766
@@ -791,7 +791,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) {
791791 cfg.setDefaults("/tmp", "")
792792
793793 env := env.NewFromMap(map[string]string{})
794− resolver := NewEnvironmentVariableResolver(env)
794+ resolver := NewShellVariableResolver(env)
795795 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
796796 require.NoError(t, err)
797797
@@ -822,7 +822,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) {
822822 cfg.setDefaults("/tmp", "")
823823
824824 env := env.NewFromMap(map[string]string{})
825− resolver := NewEnvironmentVariableResolver(env)
825+ resolver := NewShellVariableResolver(env)
826826 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
827827 require.NoError(t, err)
828828
@@ -855,7 +855,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) {
855855 env := env.NewFromMap(map[string]string{
856856 "OPENAI_API_KEY": "test-key",
857857 })
858− resolver := NewEnvironmentVariableResolver(env)
858+ resolver := NewShellVariableResolver(env)
859859 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
860860 require.NoError(t, err)
861861
@@ -889,7 +889,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {
889889 cfg := &Config{}
890890 cfg.setDefaults("/tmp", "")
891891 env := env.NewFromMap(map[string]string{})
892− resolver := NewEnvironmentVariableResolver(env)
892+ resolver := NewShellVariableResolver(env)
893893 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
894894 require.NoError(t, err)
895895
@@ -925,7 +925,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {
925925 cfg := &Config{}
926926 cfg.setDefaults("/tmp", "")
927927 env := env.NewFromMap(map[string]string{})
928− resolver := NewEnvironmentVariableResolver(env)
928+ resolver := NewShellVariableResolver(env)
929929 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
930930 require.NoError(t, err)
931931
@@ -955,7 +955,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {
955955 cfg := &Config{}
956956 cfg.setDefaults("/tmp", "")
957957 env := env.NewFromMap(map[string]string{})
958− resolver := NewEnvironmentVariableResolver(env)
958+ resolver := NewShellVariableResolver(env)
959959 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
960960 require.NoError(t, err)
961961 _, _, err = cfg.defaultModelSelection(knownProviders)
@@ -998,7 +998,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {
998998 }
999999 cfg.setDefaults("/tmp", "")
10001000 env := env.NewFromMap(map[string]string{})
1001− resolver := NewEnvironmentVariableResolver(env)
1001+ resolver := NewShellVariableResolver(env)
10021002 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
10031003 require.NoError(t, err)
10041004 large, small, err := cfg.defaultModelSelection(knownProviders)
@@ -1042,7 +1042,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {
10421042 }
10431043 cfg.setDefaults("/tmp", "")
10441044 env := env.NewFromMap(map[string]string{})
1045− resolver := NewEnvironmentVariableResolver(env)
1045+ resolver := NewShellVariableResolver(env)
10461046 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
10471047 require.NoError(t, err)
10481048 _, _, err = cfg.defaultModelSelection(knownProviders)
@@ -1084,7 +1084,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {
10841084 }
10851085 cfg.setDefaults("/tmp", "")
10861086 env := env.NewFromMap(map[string]string{})
1087− resolver := NewEnvironmentVariableResolver(env)
1087+ resolver := NewShellVariableResolver(env)
10881088 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
10891089 require.NoError(t, err)
10901090 large, small, err := cfg.defaultModelSelection(knownProviders)
@@ -1129,7 +1129,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {
11291129 env := env.NewFromMap(map[string]string{
11301130 "OPENAI_API_KEY": "test-key",
11311131 })
1132− resolver := NewEnvironmentVariableResolver(env)
1132+ resolver := NewShellVariableResolver(env)
11331133 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
11341134 require.ErrorContains(t, err, "no custom providers")
11351135
@@ -1172,7 +1172,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {
11721172 "MY_API_KEY": "test-key",
11731173 "OPENAI_API_KEY": "test-key",
11741174 })
1175− resolver := NewEnvironmentVariableResolver(env)
1175+ resolver := NewShellVariableResolver(env)
11761176 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
11771177 require.NoError(t, err)
11781178
@@ -1226,7 +1226,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {
12261226 "OPENAI_API_KEY": "test-key",
12271227 "ANTHROPIC_API_KEY": "test-key",
12281228 })
1229− resolver := NewEnvironmentVariableResolver(env)
1229+ resolver := NewShellVariableResolver(env)
12301230 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
12311231 require.NoError(t, err)
12321232
@@ -1254,7 +1254,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {
12541254 cfg.setDefaults("/tmp", "")
12551255
12561256 env := env.NewFromMap(map[string]string{})
1257− resolver := NewEnvironmentVariableResolver(env)
1257+ resolver := NewShellVariableResolver(env)
12581258 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})
12591259 require.ErrorContains(t, err, "no custom providers")
12601260
@@ -1278,7 +1278,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {
12781278 cfg.setDefaults("/tmp", "")
12791279
12801280 env := env.NewFromMap(map[string]string{})
1281− resolver := NewEnvironmentVariableResolver(env)
1281+ resolver := NewShellVariableResolver(env)
12821282 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})
12831283 require.ErrorContains(t, err, "no custom providers")
12841284
@@ -1336,7 +1336,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {
13361336 cfg.setDefaults(dir, "")
13371337 store := &ConfigStore{config: cfg, globalDataPath: globalPath}
13381338 env := env.NewFromMap(map[string]string{})
1339− resolver := NewEnvironmentVariableResolver(env)
1339+ resolver := NewShellVariableResolver(env)
13401340 err := cfg.configureProviders(store, env, resolver, knownProviders)
13411341 require.NoError(t, err)
13421342
@@ -1386,7 +1386,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {
13861386 }
13871387 cfg.setDefaults("/tmp", "")
13881388 env := env.NewFromMap(map[string]string{})
1389− resolver := NewEnvironmentVariableResolver(env)
1389+ resolver := NewShellVariableResolver(env)
13901390 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
13911391 require.NoError(t, err)
13921392
@@ -1448,7 +1448,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {
14481448 }
14491449 cfg.setDefaults("/tmp", "")
14501450 env := env.NewFromMap(map[string]string{})
1451− resolver := NewEnvironmentVariableResolver(env)
1451+ resolver := NewShellVariableResolver(env)
14521452 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
14531453 require.NoError(t, err)
14541454
@@ -1493,7 +1493,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {
14931493 }
14941494 cfg.setDefaults("/tmp", "")
14951495 env := env.NewFromMap(map[string]string{})
1496− resolver := NewEnvironmentVariableResolver(env)
1496+ resolver := NewShellVariableResolver(env)
14971497 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
14981498 require.NoError(t, err)
14991499
@@ -1532,7 +1532,7 @@ func TestConfig_configureProviders_HyperAPIKeyFromEnv(t *testing.T) {
15321532 env := env.NewFromMap(map[string]string{
15331533 "HYPER_API_KEY": "env-api-key",
15341534 })
1535− resolver := NewEnvironmentVariableResolver(env)
1535+ resolver := NewShellVariableResolver(env)
15361536 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
15371537 require.NoError(t, err)
15381538 require.Equal(t, 1, cfg.Providers.Len())
@@ -1579,7 +1579,7 @@ func TestConfig_configureProviders_HyperAPIKeyFromConfigOverrides(t *testing.T)
15791579 env := env.NewFromMap(map[string]string{
15801580 "HYPER_API_KEY": "env-api-key",
15811581 })
1582− resolver := NewEnvironmentVariableResolver(env)
1582+ resolver := NewShellVariableResolver(env)
15831583 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)
15841584 require.NoError(t, err)
15851585 require.Equal(t, 1, cfg.Providers.Len())
internal/config/resolve.go+1−26
@@ -10,8 +10,7 @@ import (
1010)
1111
1212// resolveTimeout bounds how long a single ResolveValue call may spend
13−// inside shell expansion (including any command substitution). Matches
14−// the timeout used by the previous hand-rolled parser.
13+// inside shell expansion (including any command substitution).
1514const resolveTimeout = 5 * time.Minute
1615
1716type VariableResolver interface {
@@ -173,27 +172,3 @@ func scrubErrorMessage(s string) string {
173172 }
174173 return string(out)
175174}
176−
177−type environmentVariableResolver struct {
178− env env.Env
179−}
180−
181−func NewEnvironmentVariableResolver(env env.Env) VariableResolver {
182− return &environmentVariableResolver{
183− env: env,
184− }
185−}
186−
187−// ResolveValue resolves environment variables from the provided env.Env.
188−func (r *environmentVariableResolver) ResolveValue(value string) (string, error) {
189− if len(value) == 0 || value[0] != '$' {
190− return value, nil
191− }
192−
193− varName := value[1:]
194− resolvedValue := r.env.Get(varName)
195− if resolvedValue == "" {
196− return "", fmt.Errorf("environment variable %q not set", varName)
197− }
198− return resolvedValue, nil
199−}
internal/config/resolve_test.go−64
@@ -220,62 +220,6 @@ func TestScrubErrorMessage(t *testing.T) {
220220 })
221221}
222222
223−func TestEnvironmentVariableResolver_ResolveValue(t *testing.T) {
224− tests := []struct {
225− name string
226− value string
227− envVars map[string]string
228− expected string
229− expectError bool
230− }{
231− {
232− name: "non-variable string returns as-is",
233− value: "plain-string",
234− expected: "plain-string",
235− },
236− {
237− name: "environment variable resolution",
238− value: "$HOME",
239− envVars: map[string]string{"HOME": "/home/user"},
240− expected: "/home/user",
241− },
242− {
243− name: "environment variable with complex value",
244− value: "$PATH",
245− envVars: map[string]string{"PATH": "/usr/bin:/bin:/usr/local/bin"},
246− expected: "/usr/bin:/bin:/usr/local/bin",
247− },
248− {
249− name: "missing environment variable returns error",
250− value: "$MISSING_VAR",
251− envVars: map[string]string{},
252− expectError: true,
253− },
254− {
255− name: "empty environment variable returns error",
256− value: "$EMPTY_VAR",
257− envVars: map[string]string{"EMPTY_VAR": ""},
258− expectError: true,
259− },
260− }
261−
262− for _, tt := range tests {
263− t.Run(tt.name, func(t *testing.T) {
264− testEnv := env.NewFromMap(tt.envVars)
265− resolver := NewEnvironmentVariableResolver(testEnv)
266−
267− result, err := resolver.ResolveValue(tt.value)
268−
269− if tt.expectError {
270− require.Error(t, err)
271− } else {
272− require.NoError(t, err)
273− require.Equal(t, tt.expected, result)
274− }
275− })
276− }
277−}
278−
279223func TestNewShellVariableResolver(t *testing.T) {
280224 testEnv := env.NewFromMap(map[string]string{"TEST": "value"})
281225 resolver := NewShellVariableResolver(testEnv)
@@ -283,11 +227,3 @@ func TestNewShellVariableResolver(t *testing.T) {
283227 require.NotNil(t, resolver)
284228 require.Implements(t, (*VariableResolver)(nil), resolver)
285229}
286−
287−func TestNewEnvironmentVariableResolver(t *testing.T) {
288− testEnv := env.NewFromMap(map[string]string{"TEST": "value"})
289− resolver := NewEnvironmentVariableResolver(testEnv)
290−
291− require.NotNil(t, resolver)
292− require.Implements(t, (*VariableResolver)(nil), resolver)
293−}
internal/lsp/client_test.go+1−1
@@ -26,7 +26,7 @@ func TestClient(t *testing.T) {
2626
2727 // Test creating a powernap client - this will likely fail with echo
2828 // but we can still test the basic structure
29− client, err := New(ctx, "test", cfg, config.NewEnvironmentVariableResolver(env.NewFromMap(map[string]string{
29+ client, err := New(ctx, "test", cfg, config.NewShellVariableResolver(env.NewFromMap(map[string]string{
3030 "THE_CMD": "echo",
3131 })), ".", false)
3232 if err != nil {