Skip to content

Commit ad0be26

Browse files
jkyberneeesclaude
andauthored
fix(config): isolate global state in loader tests to fix order-dependent failures (#15)
The internal/config tests failed under a full-package run but passed in isolation. Tests mutated process-global state — os.Setenv("ODEK_*"/"HOME"), os.Chdir — with hand-written defer cleanup. The deeper leak: LoadConfig calls loadSecretsEnv(), which reads the real ~/.odek/secrets.env and os.Setenv's its contents (e.g. ODEK_MODEL) into the process for any test that did not isolate $HOME. Those loader-side os.Setenv calls are not restored by test defers and leaked into later tests, where the env > file precedence overrode file/default config and broke assertions (e.g. Model = "deepseek-v4-flash"). Test-only fix (loader.go behavior unchanged): - Replace os.Setenv + defer os.Unsetenv/os.Setenv with t.Setenv (auto-restored, leak-proof, panics under t.Parallel). - Replace os.Chdir + defer os.Chdir(cwd) with t.Chdir. - Point $HOME at an isolated t.TempDir() in every LoadConfig test so loadSecretsEnv never reads the developer's real secrets.env. Verified: gofmt clean, go vet clean, go test ./internal/config/... -count=1 passes, and passes under -shuffle=on across seeds 1/42/99/7777. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 0223290 commit ad0be26

1 file changed

Lines changed: 59 additions & 110 deletions

File tree

internal/config/loader_test.go

Lines changed: 59 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ func TestLoadConfig_Defaults(t *testing.T) {
3535
}
3636

3737
func TestLoadConfig_CLIOnly(t *testing.T) {
38+
t.Setenv("HOME", t.TempDir())
3839
cfg := LoadConfig(CLIFlags{
3940
Model: "gpt-4o",
4041
BaseURL: "https://api.openai.com/v1",
@@ -72,14 +73,11 @@ func TestLoadConfig_CLIOnly(t *testing.T) {
7273
}
7374

7475
func TestLoadConfig_CLIOverridesEnv(t *testing.T) {
75-
os.Setenv("ODEK_MODEL", "env-model")
76-
os.Setenv("ODEK_BASE_URL", "https://env.example.com/v1")
77-
os.Setenv("ODEK_THINKING", "low")
78-
os.Setenv("ODEK_SANDBOX", "true")
79-
defer os.Unsetenv("ODEK_MODEL")
80-
defer os.Unsetenv("ODEK_BASE_URL")
81-
defer os.Unsetenv("ODEK_THINKING")
82-
defer os.Unsetenv("ODEK_SANDBOX")
76+
t.Setenv("HOME", t.TempDir())
77+
t.Setenv("ODEK_MODEL", "env-model")
78+
t.Setenv("ODEK_BASE_URL", "https://env.example.com/v1")
79+
t.Setenv("ODEK_THINKING", "low")
80+
t.Setenv("ODEK_SANDBOX", "true")
8381

8482
cfg := LoadConfig(CLIFlags{
8583
Model: "cli-model",
@@ -100,26 +98,16 @@ func TestLoadConfig_CLIOverridesEnv(t *testing.T) {
10098
}
10199

102100
func TestLoadConfig_EnvVars(t *testing.T) {
103-
os.Setenv("ODEK_MODEL", "deepseek-v4-flash")
104-
os.Setenv("ODEK_BASE_URL", "https://custom.deepseek.com/v1")
105-
os.Setenv("ODEK_API_KEY", "sk-env-key")
106-
os.Setenv("ODEK_THINKING", "enabled")
107-
os.Setenv("ODEK_MAX_ITER", "50")
108-
os.Setenv("ODEK_SANDBOX", "true")
109-
os.Setenv("ODEK_NO_COLOR", "false")
110-
os.Setenv("ODEK_NO_AGENTS", "true")
111-
os.Setenv("ODEK_SYSTEM", "Env system prompt.")
112-
defer func() {
113-
os.Unsetenv("ODEK_MODEL")
114-
os.Unsetenv("ODEK_BASE_URL")
115-
os.Unsetenv("ODEK_API_KEY")
116-
os.Unsetenv("ODEK_THINKING")
117-
os.Unsetenv("ODEK_MAX_ITER")
118-
os.Unsetenv("ODEK_SANDBOX")
119-
os.Unsetenv("ODEK_NO_COLOR")
120-
os.Unsetenv("ODEK_NO_AGENTS")
121-
os.Unsetenv("ODEK_SYSTEM")
122-
}()
101+
t.Setenv("HOME", t.TempDir())
102+
t.Setenv("ODEK_MODEL", "deepseek-v4-flash")
103+
t.Setenv("ODEK_BASE_URL", "https://custom.deepseek.com/v1")
104+
t.Setenv("ODEK_API_KEY", "sk-env-key")
105+
t.Setenv("ODEK_THINKING", "enabled")
106+
t.Setenv("ODEK_MAX_ITER", "50")
107+
t.Setenv("ODEK_SANDBOX", "true")
108+
t.Setenv("ODEK_NO_COLOR", "false")
109+
t.Setenv("ODEK_NO_AGENTS", "true")
110+
t.Setenv("ODEK_SYSTEM", "Env system prompt.")
123111

124112
cfg := LoadConfig(CLIFlags{})
125113
if cfg.Model != "deepseek-v4-flash" {
@@ -174,10 +162,9 @@ func TestLoadConfig_APIKeyFallback_OpenAI(t *testing.T) {
174162
}
175163

176164
func TestLoadConfig_APIKey_KODEOverridesLegacy(t *testing.T) {
177-
os.Setenv("ODEK_API_KEY", "sk-odek")
178-
os.Setenv("DEEPSEEK_API_KEY", "sk-deepseek")
179-
defer os.Unsetenv("ODEK_API_KEY")
180-
defer os.Unsetenv("DEEPSEEK_API_KEY")
165+
t.Setenv("HOME", t.TempDir())
166+
t.Setenv("ODEK_API_KEY", "sk-odek")
167+
t.Setenv("DEEPSEEK_API_KEY", "sk-deepseek")
181168

182169
cfg := LoadConfig(CLIFlags{})
183170
if cfg.APIKey != "sk-odek" {
@@ -186,10 +173,9 @@ func TestLoadConfig_APIKey_KODEOverridesLegacy(t *testing.T) {
186173
}
187174

188175
func TestLoadConfig_EnvBoolParsing(t *testing.T) {
189-
os.Setenv("ODEK_SANDBOX", "1")
190-
os.Setenv("ODEK_NO_COLOR", "0")
191-
defer os.Unsetenv("ODEK_SANDBOX")
192-
defer os.Unsetenv("ODEK_NO_COLOR")
176+
t.Setenv("HOME", t.TempDir())
177+
t.Setenv("ODEK_SANDBOX", "1")
178+
t.Setenv("ODEK_NO_COLOR", "0")
193179

194180
cfg := LoadConfig(CLIFlags{})
195181
if !cfg.Sandbox {
@@ -202,9 +188,7 @@ func TestLoadConfig_EnvBoolParsing(t *testing.T) {
202188

203189
func TestLoadConfig_GlobalFile(t *testing.T) {
204190
dir := t.TempDir()
205-
prevHome := os.Getenv("HOME")
206-
os.Setenv("HOME", dir)
207-
defer os.Setenv("HOME", prevHome)
191+
t.Setenv("HOME", dir)
208192

209193
// Create ~/.odek/config.json
210194
cfgDir := filepath.Join(dir, ".odek")
@@ -242,9 +226,7 @@ func TestLoadConfig_ProjectOverridesGlobal(t *testing.T) {
242226
dir := t.TempDir()
243227

244228
// Set HOME to temp dir for global config
245-
prevHome := os.Getenv("HOME")
246-
os.Setenv("HOME", dir)
247-
defer os.Setenv("HOME", prevHome)
229+
t.Setenv("HOME", dir)
248230

249231
// Create ~/.odek/config.json (global)
250232
globalDir := filepath.Join(dir, ".odek")
@@ -259,9 +241,7 @@ func TestLoadConfig_ProjectOverridesGlobal(t *testing.T) {
259241
}
260242

261243
// Create ./odek.json in temp dir (project)
262-
cwd, _ := os.Getwd()
263-
os.Chdir(dir)
264-
defer os.Chdir(cwd)
244+
t.Chdir(dir)
265245

266246
if err := os.WriteFile(filepath.Join(dir, "odek.json"), []byte(`{
267247
"model": "project-model",
@@ -283,10 +263,9 @@ func TestLoadConfig_ProjectOverridesGlobal(t *testing.T) {
283263
}
284264

285265
func TestLoadConfig_EnvOverridesProjectFile(t *testing.T) {
266+
t.Setenv("HOME", t.TempDir())
286267
dir := t.TempDir()
287-
cwd, _ := os.Getwd()
288-
os.Chdir(dir)
289-
defer os.Chdir(cwd)
268+
t.Chdir(dir)
290269

291270
// Create ./odek.json
292271
if err := os.WriteFile(filepath.Join(dir, "odek.json"), []byte(`{
@@ -297,8 +276,7 @@ func TestLoadConfig_EnvOverridesProjectFile(t *testing.T) {
297276
}
298277

299278
// Set env vars
300-
os.Setenv("ODEK_MODEL", "env-model")
301-
defer os.Unsetenv("ODEK_MODEL")
279+
t.Setenv("ODEK_MODEL", "env-model")
302280

303281
cfg := LoadConfig(CLIFlags{})
304282
if cfg.Model != "env-model" {
@@ -310,10 +288,9 @@ func TestLoadConfig_EnvOverridesProjectFile(t *testing.T) {
310288
}
311289

312290
func TestLoadConfig_CLIOverridesProjectFile(t *testing.T) {
291+
t.Setenv("HOME", t.TempDir())
313292
dir := t.TempDir()
314-
cwd, _ := os.Getwd()
315-
os.Chdir(dir)
316-
defer os.Chdir(cwd)
293+
t.Chdir(dir)
317294

318295
// Create ./odek.json
319296
if err := os.WriteFile(filepath.Join(dir, "odek.json"), []byte(`{
@@ -338,14 +315,10 @@ func TestLoadConfig_CLIOverridesProjectFile(t *testing.T) {
338315
func TestLoadConfig_VarExpansion(t *testing.T) {
339316
dir := t.TempDir()
340317

341-
prevHome := os.Getenv("HOME")
342-
os.Setenv("HOME", dir)
343-
defer os.Setenv("HOME", prevHome)
318+
t.Setenv("HOME", dir)
344319

345-
os.Setenv("ODEK_MODEL_VAR", "expanded-model")
346-
os.Setenv("ODEK_API_KEY_VAR", "sk-expanded")
347-
defer os.Unsetenv("ODEK_MODEL_VAR")
348-
defer os.Unsetenv("ODEK_API_KEY_VAR")
320+
t.Setenv("ODEK_MODEL_VAR", "expanded-model")
321+
t.Setenv("ODEK_API_KEY_VAR", "sk-expanded")
349322

350323
globalDir := filepath.Join(dir, ".odek")
351324
os.MkdirAll(globalDir, 0755)
@@ -368,9 +341,7 @@ func TestLoadConfig_VarExpansion(t *testing.T) {
368341
func TestLoadConfig_MissingFiles(t *testing.T) {
369342
// No files at all — should not panic, return zero values
370343
dir := t.TempDir()
371-
prevHome := os.Getenv("HOME")
372-
os.Setenv("HOME", dir)
373-
defer os.Setenv("HOME", prevHome)
344+
t.Setenv("HOME", dir)
374345

375346
// Don't create any config files
376347
cfg := LoadConfig(CLIFlags{})
@@ -381,9 +352,7 @@ func TestLoadConfig_MissingFiles(t *testing.T) {
381352

382353
func TestLoadConfig_InvalidJSON(t *testing.T) {
383354
dir := t.TempDir()
384-
prevHome := os.Getenv("HOME")
385-
os.Setenv("HOME", dir)
386-
defer os.Setenv("HOME", prevHome)
355+
t.Setenv("HOME", dir)
387356

388357
globalDir := filepath.Join(dir, ".odek")
389358
os.MkdirAll(globalDir, 0755)
@@ -423,14 +392,10 @@ func TestLoadConfig_SkillsLearnEnvDoesNotClobberSkillsConfig(t *testing.T) {
423392
// existing skills config from files, not replace the entire struct.
424393
dir := t.TempDir()
425394

426-
prevHome := os.Getenv("HOME")
427-
os.Setenv("HOME", dir)
428-
defer os.Setenv("HOME", prevHome)
395+
t.Setenv("HOME", dir)
429396

430397
// Create project file with skills settings
431-
cwd, _ := os.Getwd()
432-
os.Chdir(dir)
433-
defer os.Chdir(cwd)
398+
t.Chdir(dir)
434399

435400
if err := os.WriteFile(filepath.Join(dir, "odek.json"), []byte(`{
436401
"skills": {
@@ -452,8 +417,7 @@ func TestLoadConfig_SkillsLearnEnvDoesNotClobberSkillsConfig(t *testing.T) {
452417
}
453418

454419
// Set ODEK_SKILLS_LEARN — should NOT clobber other skills fields
455-
os.Setenv("ODEK_SKILLS_LEARN", "true")
456-
defer os.Unsetenv("ODEK_SKILLS_LEARN")
420+
t.Setenv("ODEK_SKILLS_LEARN", "true")
457421

458422
cfg := LoadConfig(CLIFlags{})
459423
if !cfg.Skills.Learn {
@@ -478,11 +442,10 @@ func TestLoadConfig_SkillsLearnEnvDoesNotClobberSkillsConfig(t *testing.T) {
478442

479443
func TestLoadConfig_SkillsLearnCLIDoesNotClobberSkillsConfig(t *testing.T) {
480444
// Regression: --learn CLI flag should merge, not replace.
445+
t.Setenv("HOME", t.TempDir())
481446
dir := t.TempDir()
482447

483-
cwd, _ := os.Getwd()
484-
os.Chdir(dir)
485-
defer os.Chdir(cwd)
448+
t.Chdir(dir)
486449

487450
if err := os.WriteFile(filepath.Join(dir, "odek.json"), []byte(`{
488451
"skills": {
@@ -509,6 +472,7 @@ func TestLoadConfig_SkillsLearnCLIDoesNotClobberSkillsConfig(t *testing.T) {
509472
func TestLoadConfig_MemoryDefaults(t *testing.T) {
510473
// When no memory section is configured, the resolved config must have
511474
// sensible defaults (Enabled=true, all features on).
475+
t.Setenv("HOME", t.TempDir())
512476
cfg := LoadConfig(CLIFlags{})
513477
mem := cfg.Memory
514478
if mem.Enabled == nil || !*mem.Enabled {
@@ -539,9 +503,7 @@ func TestLoadConfig_MemoryDefaults(t *testing.T) {
539503

540504
func TestLoadConfig_MemoryFromGlobalFile(t *testing.T) {
541505
dir := t.TempDir()
542-
prevHome := os.Getenv("HOME")
543-
os.Setenv("HOME", dir)
544-
defer os.Setenv("HOME", prevHome)
506+
t.Setenv("HOME", dir)
545507

546508
cfgDir := filepath.Join(dir, ".odek")
547509
os.MkdirAll(cfgDir, 0755)
@@ -585,9 +547,7 @@ func TestLoadConfig_MemoryFromGlobalFile(t *testing.T) {
585547

586548
func TestLoadConfig_MemoryProjectOverridesGlobal(t *testing.T) {
587549
dir := t.TempDir()
588-
prevHome := os.Getenv("HOME")
589-
os.Setenv("HOME", dir)
590-
defer os.Setenv("HOME", prevHome)
550+
t.Setenv("HOME", dir)
591551

592552
// Global config with memory section
593553
globalDir := filepath.Join(dir, ".odek")
@@ -603,9 +563,7 @@ func TestLoadConfig_MemoryProjectOverridesGlobal(t *testing.T) {
603563
}
604564

605565
// Project config overrides some memory fields
606-
cwd, _ := os.Getwd()
607-
os.Chdir(dir)
608-
defer os.Chdir(cwd)
566+
t.Chdir(dir)
609567

610568
if err := os.WriteFile(filepath.Join(dir, "odek.json"), []byte(`{
611569
"memory": {
@@ -693,14 +651,12 @@ func TestLoadConfig_MemoryNotSetReturnsDefaults(t *testing.T) {
693651
}
694652

695653
func TestLoadConfig_ClearsAPIKeyFromEnviron(t *testing.T) {
696-
os.Setenv("ODEK_API_KEY", "sk-odek-test")
697-
os.Setenv("DEEPSEEK_API_KEY", "sk-deepseek-test")
698-
os.Setenv("OPENAI_API_KEY", "sk-openai-test")
654+
t.Setenv("ODEK_API_KEY", "sk-odek-test")
655+
t.Setenv("DEEPSEEK_API_KEY", "sk-deepseek-test")
656+
t.Setenv("OPENAI_API_KEY", "sk-openai-test")
699657

700658
dir := t.TempDir()
701-
prevHome := os.Getenv("HOME")
702-
os.Setenv("HOME", dir)
703-
defer os.Setenv("HOME", prevHome)
659+
t.Setenv("HOME", dir)
704660

705661
cfg := LoadConfig(CLIFlags{})
706662

@@ -723,6 +679,7 @@ func TestLoadConfig_InteractionModeDefaults(t *testing.T) {
723679
// default to "engaging". Note: the user's ~/.odek/config.json may
724680
// set interaction_mode, so this test accepts any non-empty value
725681
// from the file load chain and only fails on the empty-zero case.
682+
t.Setenv("HOME", t.TempDir())
726683
cfg := LoadConfig(CLIFlags{})
727684
if cfg.InteractionMode == "" {
728685
t.Errorf("InteractionMode = %q, want non-empty default", cfg.InteractionMode)
@@ -731,8 +688,8 @@ func TestLoadConfig_InteractionModeDefaults(t *testing.T) {
731688

732689
func TestLoadConfig_InteractionModeViaEnv(t *testing.T) {
733690
// ODEK_INTERACTION_MODE should override the default.
734-
os.Setenv("ODEK_INTERACTION_MODE", "verbose")
735-
defer os.Unsetenv("ODEK_INTERACTION_MODE")
691+
t.Setenv("HOME", t.TempDir())
692+
t.Setenv("ODEK_INTERACTION_MODE", "verbose")
736693

737694
cfg := LoadConfig(CLIFlags{})
738695
if cfg.InteractionMode != "verbose" {
@@ -742,8 +699,8 @@ func TestLoadConfig_InteractionModeViaEnv(t *testing.T) {
742699

743700
func TestLoadConfig_InteractionModeViaCLI(t *testing.T) {
744701
// CLI flag should take precedence over env.
745-
os.Setenv("ODEK_INTERACTION_MODE", "engaging")
746-
defer os.Unsetenv("ODEK_INTERACTION_MODE")
702+
t.Setenv("HOME", t.TempDir())
703+
t.Setenv("ODEK_INTERACTION_MODE", "engaging")
747704

748705
cfg := LoadConfig(CLIFlags{InteractionMode: "verbose"})
749706
if cfg.InteractionMode != "verbose" {
@@ -753,6 +710,7 @@ func TestLoadConfig_InteractionModeViaCLI(t *testing.T) {
753710

754711
func TestLoadConfig_InteractionModeOff(t *testing.T) {
755712
// "off" should be accepted as a valid value via CLI.
713+
t.Setenv("HOME", t.TempDir())
756714
cfg := LoadConfig(CLIFlags{InteractionMode: "off"})
757715
if cfg.InteractionMode != "off" {
758716
t.Errorf("InteractionMode = %q, want %q", cfg.InteractionMode, "off")
@@ -778,9 +736,7 @@ func TestGlobalOverlay_MaxConcurrency(t *testing.T) {
778736
}
779737

780738
// Project config exists but does NOT set max_concurrency.
781-
cwd, _ := os.Getwd()
782-
defer os.Chdir(cwd)
783-
os.Chdir(t.TempDir())
739+
t.Chdir(t.TempDir())
784740
if err := os.WriteFile("odek.json", []byte(`{
785741
"model": "project-model"
786742
}`), 0644); err != nil {
@@ -806,9 +762,7 @@ func TestGlobalOverlay_MaxToolParallel(t *testing.T) {
806762
t.Fatal(err)
807763
}
808764

809-
cwd, _ := os.Getwd()
810-
defer os.Chdir(cwd)
811-
os.Chdir(t.TempDir())
765+
t.Chdir(t.TempDir())
812766
if err := os.WriteFile("odek.json", []byte(`{
813767
"model": "project-model"
814768
}`), 0644); err != nil {
@@ -834,9 +788,7 @@ func TestGlobalOverlay_PromptCaching(t *testing.T) {
834788
t.Fatal(err)
835789
}
836790

837-
cwd, _ := os.Getwd()
838-
defer os.Chdir(cwd)
839-
os.Chdir(t.TempDir())
791+
t.Chdir(t.TempDir())
840792
if err := os.WriteFile("odek.json", []byte(`{
841793
"model": "project-model"
842794
}`), 0644); err != nil {
@@ -867,9 +819,7 @@ func TestGlobalOverlay_MCPServers(t *testing.T) {
867819
t.Fatal(err)
868820
}
869821

870-
cwd, _ := os.Getwd()
871-
defer os.Chdir(cwd)
872-
os.Chdir(t.TempDir())
822+
t.Chdir(t.TempDir())
873823
if err := os.WriteFile("odek.json", []byte(`{
874824
"model": "project-model"
875825
}`), 0644); err != nil {
@@ -898,8 +848,7 @@ func TestGlobalOverlay_MCPServers(t *testing.T) {
898848
// forms (ODEK_API_KEY, DEEPSEEK_API_KEY, OPENAI_API_KEY) from the resolved key.
899849
func TestLoadConfig_LegacyAPIKeyEnvVarLost(t *testing.T) {
900850
// Set only the legacy DEEPSEEK_API_KEY — no ODEK_API_KEY, no config file.
901-
os.Setenv("DEEPSEEK_API_KEY", "sk-deepseek-only")
902-
defer os.Unsetenv("DEEPSEEK_API_KEY")
851+
t.Setenv("DEEPSEEK_API_KEY", "sk-deepseek-only")
903852

904853
t.Setenv("HOME", t.TempDir())
905854

0 commit comments

Comments
 (0)