fix(config): isolate global state in loader tests to fix order-dependent failures#15
Conversation
…ent failures
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>
Verification CertificateProtocol: AI Verification Protocol v5.2.7 (single-reviewer application — no multi-agent pipeline; Axes Summary
η (informal, single-reviewer)Oracle is strong and independent of the change: the same production Verification Debt
VerdictHumanReviewRecommended — mechanically clean and fully reproduced, but capped below AutoApprove because this is a single-reviewer certificate, not the protocol's required multi-provider pipeline. Recommended for merge. Rationale: Root cause (loader-side 🤖 Generated with Claude Code |
Summary
go test ./internal/config/... -count=1failed, but the same tests passed in isolation. This was a pre-existing test-isolation defect onmain(reproduced via a clean worktree ofmain), unrelated to any recent config change.Example failure:
loader_test.go: Model = "deepseek-v4-flash", want empty— a value that only appears as theTestLoadConfig_EnvVarsfixture.Root cause
Two compounding sources of leaked process-global state:
os.Setenv("ODEK_*"/"HOME", …)andos.Chdir(…)with hand-writtendefercleanup that is ordering-fragile.LoadConfig→loadSecretsEnv()reads the developer's real~/.odek/secrets.envandos.Setenvs its contents (e.g.ODEK_MODEL=deepseek-v4-flash) into the process for any test that did not isolate$HOME. Those loader-sideos.Setenvcalls are not covered by any testdefer, so they leaked into later tests, where the documentedenv > fileprecedence overrode file/default config and broke assertions.Fix (test-only —
loader.gobehavior unchanged)os.Setenv+defer os.Unsetenv/os.Setenvwitht.Setenv(auto-restored, leak-proof, panics undert.Parallel).os.Chdir+defer os.Chdir(cwd)witht.Chdir.$HOMEat an isolatedt.TempDir()in every test that callsLoadConfig, soloadSecretsEnvnever reads the realsecrets.envand never injects un-restorable env vars.Net:
1 file changed, 59 insertions(+), 110 deletions(-).Verification
gofmt -l internal/config/loader_test.gogo vet ./internal/config/...go test ./internal/config/... -count=1go test ./internal/config/... -count=1 -shuffle=on-shuffle=seeds 1 / 42 / 99 / 7777go build ./...🤖 Generated with Claude Code