f913477 · crush · git.iko.soy

git.iko.soy/crush

on main help sign in

config: remove unused environment variable resolver

f913477Christian Rocha6 months ago

internal/config/load_test.go+39−39

compare with main

unifiedsplit

@@ -77,7 +77,7 @@ func TestConfig_configureProviders(t *testing.T) {

77 env := env.NewFromMap(map[string]string{

78 "OPENAI_API_KEY": "test-key",

79 })

80− resolver := NewEnvironmentVariableResolver(env)

80+ resolver := NewShellVariableResolver(env)

81 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

82 require.NoError(t, err)

83 require.Equal(t, 1, cfg.Providers.Len())

@@ -120,7 +120,7 @@ func TestConfig_configureProvidersWithOverride(t *testing.T) {

120 env := env.NewFromMap(map[string]string{

121 "OPENAI_API_KEY": "test-key",

122 })

123− resolver := NewEnvironmentVariableResolver(env)

123+ resolver := NewShellVariableResolver(env)

124 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

125 require.NoError(t, err)

126 require.Equal(t, 1, cfg.Providers.Len())

@@ -162,7 +162,7 @@ func TestConfig_configureProvidersWithNewProvider(t *testing.T) {

162 env := env.NewFromMap(map[string]string{

163 "OPENAI_API_KEY": "test-key",

164 })

165− resolver := NewEnvironmentVariableResolver(env)

165+ resolver := NewShellVariableResolver(env)

166 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

167 require.NoError(t, err)

168 // Should be to because of the env variable

@@ -198,7 +198,7 @@ func TestConfig_configureProvidersBedrockWithCredentials(t *testing.T) {

198 "AWS_ACCESS_KEY_ID": "test-key-id",

199 "AWS_SECRET_ACCESS_KEY": "test-secret-key",

200 })

201− resolver := NewEnvironmentVariableResolver(env)

201+ resolver := NewShellVariableResolver(env)

202 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

203 require.NoError(t, err)

204 require.Equal(t, cfg.Providers.Len(), 1)

@@ -224,7 +224,7 @@ func TestConfig_configureProvidersBedrockWithoutCredentials(t *testing.T) {

224 cfg := &Config{}

225 cfg.setDefaults("/tmp", "")

226 env := env.NewFromMap(map[string]string{})

227− resolver := NewEnvironmentVariableResolver(env)

227+ resolver := NewShellVariableResolver(env)

228 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

229 require.NoError(t, err)

230 // Provider should not be configured without credentials

@@ -249,7 +249,7 @@ func TestConfig_configureProvidersBedrockWithoutUnsupportedModel(t *testing.T) {

249 "AWS_ACCESS_KEY_ID": "test-key-id",

250 "AWS_SECRET_ACCESS_KEY": "test-secret-key",

251 })

252− resolver := NewEnvironmentVariableResolver(env)

252+ resolver := NewShellVariableResolver(env)

253 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

254 require.Error(t, err)

255}

@@ -272,7 +272,7 @@ func TestConfig_configureProvidersVertexAIWithCredentials(t *testing.T) {

272 "VERTEXAI_PROJECT": "test-project",

273 "VERTEXAI_LOCATION": "us-central1",

274 })

275− resolver := NewEnvironmentVariableResolver(env)

275+ resolver := NewShellVariableResolver(env)

276 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

277 require.NoError(t, err)

278 require.Equal(t, cfg.Providers.Len(), 1)

@@ -304,7 +304,7 @@ func TestConfig_configureProvidersVertexAIWithoutCredentials(t *testing.T) {

304 "GOOGLE_CLOUD_PROJECT": "test-project",

305 "GOOGLE_CLOUD_LOCATION": "us-central1",

306 })

307− resolver := NewEnvironmentVariableResolver(env)

307+ resolver := NewShellVariableResolver(env)

308 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

309 require.NoError(t, err)

310 // Provider should not be configured without proper credentials

@@ -329,7 +329,7 @@ func TestConfig_configureProvidersVertexAIMissingProject(t *testing.T) {

329 "GOOGLE_GENAI_USE_VERTEXAI": "true",

330 "GOOGLE_CLOUD_LOCATION": "us-central1",

331 })

332− resolver := NewEnvironmentVariableResolver(env)

332+ resolver := NewShellVariableResolver(env)

333 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

334 require.NoError(t, err)

335 // Provider should not be configured without project

@@ -353,7 +353,7 @@ func TestConfig_configureProvidersSetProviderID(t *testing.T) {

353 env := env.NewFromMap(map[string]string{

354 "OPENAI_API_KEY": "test-key",

355 })

356− resolver := NewEnvironmentVariableResolver(env)

356+ resolver := NewShellVariableResolver(env)

357 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

358 require.NoError(t, err)

359 require.Equal(t, cfg.Providers.Len(), 1)

@@ -544,7 +544,7 @@ func TestConfig_configureProvidersWithDisabledProvider(t *testing.T) {

544 env := env.NewFromMap(map[string]string{

545 "OPENAI_API_KEY": "test-key",

546 })

547− resolver := NewEnvironmentVariableResolver(env)

547+ resolver := NewShellVariableResolver(env)

548 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

549 require.NoError(t, err)

550

@@ -572,7 +572,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {

572 cfg.setDefaults("/tmp", "")

573

574 env := env.NewFromMap(map[string]string{})

575− resolver := NewEnvironmentVariableResolver(env)

575+ resolver := NewShellVariableResolver(env)

576 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})

577 require.NoError(t, err)

578

@@ -595,7 +595,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {

595 cfg.setDefaults("/tmp", "")

596

597 env := env.NewFromMap(map[string]string{})

598− resolver := NewEnvironmentVariableResolver(env)

598+ resolver := NewShellVariableResolver(env)

599 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})

600 require.NoError(t, err)

601

@@ -617,7 +617,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {

617 cfg.setDefaults("/tmp", "")

618

619 env := env.NewFromMap(map[string]string{})

620− resolver := NewEnvironmentVariableResolver(env)

620+ resolver := NewShellVariableResolver(env)

621 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})

622 require.NoError(t, err)

623

@@ -642,7 +642,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {

642 cfg.setDefaults("/tmp", "")

643

644 env := env.NewFromMap(map[string]string{})

645− resolver := NewEnvironmentVariableResolver(env)

645+ resolver := NewShellVariableResolver(env)

646 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})

647 require.NoError(t, err)

648

@@ -667,7 +667,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {

667 cfg.setDefaults("/tmp", "")

668

669 env := env.NewFromMap(map[string]string{})

670− resolver := NewEnvironmentVariableResolver(env)

670+ resolver := NewShellVariableResolver(env)

671 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})

672 require.NoError(t, err)

673

@@ -695,7 +695,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {

695 cfg.setDefaults("/tmp", "")

696

697 env := env.NewFromMap(map[string]string{})

698− resolver := NewEnvironmentVariableResolver(env)

698+ resolver := NewShellVariableResolver(env)

699 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})

700 require.NoError(t, err)

701

@@ -725,7 +725,7 @@ func TestConfig_configureProvidersCustomProviderValidation(t *testing.T) {

725 cfg.setDefaults("/tmp", "")

726

727 env := env.NewFromMap(map[string]string{})

728− resolver := NewEnvironmentVariableResolver(env)

728+ resolver := NewShellVariableResolver(env)

729 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})

730 require.NoError(t, err)

731

@@ -760,7 +760,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) {

760 env := env.NewFromMap(map[string]string{

761 "GOOGLE_GENAI_USE_VERTEXAI": "false",

762 })

763− resolver := NewEnvironmentVariableResolver(env)

763+ resolver := NewShellVariableResolver(env)

764 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

765 require.NoError(t, err)

766

@@ -791,7 +791,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) {

791 cfg.setDefaults("/tmp", "")

792

793 env := env.NewFromMap(map[string]string{})

794− resolver := NewEnvironmentVariableResolver(env)

794+ resolver := NewShellVariableResolver(env)

795 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

796 require.NoError(t, err)

797

@@ -822,7 +822,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) {

822 cfg.setDefaults("/tmp", "")

823

824 env := env.NewFromMap(map[string]string{})

825− resolver := NewEnvironmentVariableResolver(env)

825+ resolver := NewShellVariableResolver(env)

826 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

827 require.NoError(t, err)

828

@@ -855,7 +855,7 @@ func TestConfig_configureProvidersEnhancedCredentialValidation(t *testing.T) {

855 env := env.NewFromMap(map[string]string{

856 "OPENAI_API_KEY": "test-key",

857 })

858− resolver := NewEnvironmentVariableResolver(env)

858+ resolver := NewShellVariableResolver(env)

859 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

860 require.NoError(t, err)

861

@@ -889,7 +889,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {

889 cfg := &Config{}

890 cfg.setDefaults("/tmp", "")

891 env := env.NewFromMap(map[string]string{})

892− resolver := NewEnvironmentVariableResolver(env)

892+ resolver := NewShellVariableResolver(env)

893 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

894 require.NoError(t, err)

895

@@ -925,7 +925,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {

925 cfg := &Config{}

926 cfg.setDefaults("/tmp", "")

927 env := env.NewFromMap(map[string]string{})

928− resolver := NewEnvironmentVariableResolver(env)

928+ resolver := NewShellVariableResolver(env)

929 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

930 require.NoError(t, err)

931

@@ -955,7 +955,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {

955 cfg := &Config{}

956 cfg.setDefaults("/tmp", "")

957 env := env.NewFromMap(map[string]string{})

958− resolver := NewEnvironmentVariableResolver(env)

958+ resolver := NewShellVariableResolver(env)

959 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

960 require.NoError(t, err)

961 _, _, err = cfg.defaultModelSelection(knownProviders)

@@ -998,7 +998,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {

998 }

999 cfg.setDefaults("/tmp", "")

1000 env := env.NewFromMap(map[string]string{})

1001− resolver := NewEnvironmentVariableResolver(env)

1001+ resolver := NewShellVariableResolver(env)

1002 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1003 require.NoError(t, err)

1004 large, small, err := cfg.defaultModelSelection(knownProviders)

@@ -1042,7 +1042,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {

1042 }

1043 cfg.setDefaults("/tmp", "")

1044 env := env.NewFromMap(map[string]string{})

1045− resolver := NewEnvironmentVariableResolver(env)

1045+ resolver := NewShellVariableResolver(env)

1046 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1047 require.NoError(t, err)

1048 _, _, err = cfg.defaultModelSelection(knownProviders)

@@ -1084,7 +1084,7 @@ func TestConfig_defaultModelSelection(t *testing.T) {

1084 }

1085 cfg.setDefaults("/tmp", "")

1086 env := env.NewFromMap(map[string]string{})

1087− resolver := NewEnvironmentVariableResolver(env)

1087+ resolver := NewShellVariableResolver(env)

1088 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1089 require.NoError(t, err)

1090 large, small, err := cfg.defaultModelSelection(knownProviders)

@@ -1129,7 +1129,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {

1129 env := env.NewFromMap(map[string]string{

1130 "OPENAI_API_KEY": "test-key",

1131 })

1132− resolver := NewEnvironmentVariableResolver(env)

1132+ resolver := NewShellVariableResolver(env)

1133 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1134 require.ErrorContains(t, err, "no custom providers")

1135

@@ -1172,7 +1172,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {

1172 "MY_API_KEY": "test-key",

1173 "OPENAI_API_KEY": "test-key",

1174 })

1175− resolver := NewEnvironmentVariableResolver(env)

1175+ resolver := NewShellVariableResolver(env)

1176 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1177 require.NoError(t, err)

1178

@@ -1226,7 +1226,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {

1226 "OPENAI_API_KEY": "test-key",

1227 "ANTHROPIC_API_KEY": "test-key",

1228 })

1229− resolver := NewEnvironmentVariableResolver(env)

1229+ resolver := NewShellVariableResolver(env)

1230 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1231 require.NoError(t, err)

1232

@@ -1254,7 +1254,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {

1254 cfg.setDefaults("/tmp", "")

1255

1256 env := env.NewFromMap(map[string]string{})

1257− resolver := NewEnvironmentVariableResolver(env)

1257+ resolver := NewShellVariableResolver(env)

1258 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})

1259 require.ErrorContains(t, err, "no custom providers")

1260

@@ -1278,7 +1278,7 @@ func TestConfig_configureProvidersDisableDefaultProviders(t *testing.T) {

1278 cfg.setDefaults("/tmp", "")

1279

1280 env := env.NewFromMap(map[string]string{})

1281− resolver := NewEnvironmentVariableResolver(env)

1281+ resolver := NewShellVariableResolver(env)

1282 err := cfg.configureProviders(testStore(cfg), env, resolver, []catwalk.Provider{})

1283 require.ErrorContains(t, err, "no custom providers")

1284

@@ -1336,7 +1336,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {

1336 cfg.setDefaults(dir, "")

1337 store := &ConfigStore{config: cfg, globalDataPath: globalPath}

1338 env := env.NewFromMap(map[string]string{})

1339− resolver := NewEnvironmentVariableResolver(env)

1339+ resolver := NewShellVariableResolver(env)

1340 err := cfg.configureProviders(store, env, resolver, knownProviders)

1341 require.NoError(t, err)

1342

@@ -1386,7 +1386,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {

1386 }

1387 cfg.setDefaults("/tmp", "")

1388 env := env.NewFromMap(map[string]string{})

1389− resolver := NewEnvironmentVariableResolver(env)

1389+ resolver := NewShellVariableResolver(env)

1390 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1391 require.NoError(t, err)

1392

@@ -1448,7 +1448,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {

1448 }

1449 cfg.setDefaults("/tmp", "")

1450 env := env.NewFromMap(map[string]string{})

1451− resolver := NewEnvironmentVariableResolver(env)

1451+ resolver := NewShellVariableResolver(env)

1452 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1453 require.NoError(t, err)

1454

@@ -1493,7 +1493,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {

1493 }

1494 cfg.setDefaults("/tmp", "")

1495 env := env.NewFromMap(map[string]string{})

1496− resolver := NewEnvironmentVariableResolver(env)

1496+ resolver := NewShellVariableResolver(env)

1497 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1498 require.NoError(t, err)

1499

@@ -1532,7 +1532,7 @@ func TestConfig_configureProviders_HyperAPIKeyFromEnv(t *testing.T) {

1532 env := env.NewFromMap(map[string]string{

1533 "HYPER_API_KEY": "env-api-key",

1534 })

1535− resolver := NewEnvironmentVariableResolver(env)

1535+ resolver := NewShellVariableResolver(env)

1536 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1537 require.NoError(t, err)

1538 require.Equal(t, 1, cfg.Providers.Len())

@@ -1579,7 +1579,7 @@ func TestConfig_configureProviders_HyperAPIKeyFromConfigOverrides(t *testing.T)

1579 env := env.NewFromMap(map[string]string{

1580 "HYPER_API_KEY": "env-api-key",

1581 })

1582− resolver := NewEnvironmentVariableResolver(env)

1582+ resolver := NewShellVariableResolver(env)

1583 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1584 require.NoError(t, err)

1585 require.Equal(t, 1, cfg.Providers.Len())

internal/config/resolve.go+1−26

@@ -10,8 +10,7 @@ import (

10)

12// resolveTimeout bounds how long a single ResolveValue call may spend

13−// inside shell expansion (including any command substitution). Matches

13+// inside shell expansion (including any command substitution).

14−// the timeout used by the previous hand-rolled parser.

15const resolveTimeout = 5 * time.Minute

14const resolveTimeout = 5 * time.Minute

17type VariableResolver interface {

16type VariableResolver interface {

@@ -173,27 +172,3 @@ func scrubErrorMessage(s string) string {

173 }

172 }

174 return string(out)

173 return string(out)

175}

174}

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) {

220 })

221}

222

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−

279func TestNewShellVariableResolver(t *testing.T) {

223func TestNewShellVariableResolver(t *testing.T) {

280 testEnv := env.NewFromMap(map[string]string{"TEST": "value"})

224 testEnv := env.NewFromMap(map[string]string{"TEST": "value"})

281 resolver := NewShellVariableResolver(testEnv)

225 resolver := NewShellVariableResolver(testEnv)

@@ -283,11 +227,3 @@ func TestNewShellVariableResolver(t *testing.T) {

283 require.NotNil(t, resolver)

227 require.NotNil(t, resolver)

284 require.Implements(t, (*VariableResolver)(nil), resolver)

228 require.Implements(t, (*VariableResolver)(nil), resolver)

285}

229}

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) {

27 // Test creating a powernap client - this will likely fail with echo

28 // 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{

30 "THE_CMD": "echo",

31 })), ".", false)

32 if err != nil {