fix: resolve test isolation failures in config tests

[vsumner]

Summary

Fixes 19 failing config tests on main caused by environment variable pollution and mutex poisoning issues.

Problem

Tests in src/config.rs were failing due to:

1. Environment variable pollution: ANTHROPIC_BASE_URL from shell environment was leaking into tests that expected the default Anthropic URL
2. Mutex poisoning: std::sync::Mutex gets poisoned when tests panic, causing all subsequent tests to fail when acquiring the lock

Changes

• Added parking_lot dev-dependency: Provides Mutex that doesn't poison on panic
• Updated EnvGuard: Added ANTHROPIC_BASE_URL to the list of env vars to clear (27 total now)
• Simplified lock acquisition: parking_lot's lock() returns directly (no Result wrapper)
• Added missing isolation: Two tests (test_legacy_llm_keys_auto_migrate_to_providers, all_shorthand_keys_register_providers_via_toml) now use EnvGuard

Verification

$ cargo test --lib
...
test result: ok. 301 passed; 0 failed; 0 ignored

$ cargo clippy --lib -- -D warnings
    Finished dev profile [unoptimized] [x] Finishing feature: + polished
target(s) in 0.04s

Related

• Closes: .github/issues/test-isolation-failures.md

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

Changes to src/config.rs switch test module synchronization from std::sync::Mutex<()> to parking_lot::Mutex<()> and update lock call patterns. The KEYS array and environment variable helper are expanded to include an additional provider-related environment variable.

Changes

Cohort / File(s)	Summary
Test Module Mutex Refactoring src/config.rs	Replaces std::sync::Mutex with parking_lot::Mutex in test module, updates lock().unwrap() calls to lock(), and expands KEYS array with additional environment variable. Includes synchronization notes between KEYS and provider env vars.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: resolve test isolation failures in config tests' accurately describes the main change—fixing test isolation issues in config tests by addressing mutex poisoning and environment variable pollution.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, detailing the problems solved (environment variable pollution and mutex poisoning), specific changes made, and verification results.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/config.rs (1)

6694-6721: ⚠️ Potential issue | 🟠 Major

Include ANTHROPIC_AUTH_TOKEN in EnvGuard cleanup.

EnvGuard::new() clears ANTHROPIC_API_KEY and ANTHROPIC_BASE_URL, but not ANTHROPIC_AUTH_TOKEN. Since config loading reads ANTHROPIC_AUTH_TOKEN (Line 3756 and Line 4429), shell state can still leak into tests.

Proposed fix

-            const KEYS: [&str; 27] = [
+            const KEYS: [&str; 28] = [
                 "SPACEBOT_DIR",
                 "SPACEBOT_DEPLOYMENT",
                 "SPACEBOT_CRON_TIMEZONE",
                 "SPACEBOT_USER_TIMEZONE",
                 "ANTHROPIC_API_KEY",
                 "ANTHROPIC_BASE_URL",
+                "ANTHROPIC_AUTH_TOKEN",
                 "ANTHROPIC_OAUTH_TOKEN",
                 "OPENAI_API_KEY",
                 "OPENROUTER_API_KEY",
                 "KILO_API_KEY",
                 "ZHIPU_API_KEY",
                 "GROQ_API_KEY",
                 "TOGETHER_API_KEY",
                 "FIREWORKS_API_KEY",
                 "DEEPSEEK_API_KEY",
                 "XAI_API_KEY",
                 "MISTRAL_API_KEY",
                 "GEMINI_API_KEY",
                 "NVIDIA_API_KEY",
                 "OLLAMA_API_KEY",
                 "OLLAMA_BASE_URL",
                 "OPENCODE_ZEN_API_KEY",
                 "OPENCODE_GO_API_KEY",
                 "MINIMAX_API_KEY",
                 "MINIMAX_CN_API_KEY",
                 "MOONSHOT_API_KEY",
                 "ZAI_CODING_PLAN_API_KEY",
             ];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 6694 - 6721, EnvGuard::new() currently clears
environment keys listed in the KEYS array but omits "ANTHROPIC_AUTH_TOKEN",
allowing that value to leak into tests; update the KEYS constant inside
EnvGuard::new() to include "ANTHROPIC_AUTH_TOKEN" so the guard clears that
variable as well (this directly ties to EnvGuard and the config code paths that
read ANTHROPIC_AUTH_TOKEN during config loading).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/config.rs`:
- Around line 6694-6721: EnvGuard::new() currently clears environment keys
listed in the KEYS array but omits "ANTHROPIC_AUTH_TOKEN", allowing that value
to leak into tests; update the KEYS constant inside EnvGuard::new() to include
"ANTHROPIC_AUTH_TOKEN" so the guard clears that variable as well (this directly
ties to EnvGuard and the config code paths that read ANTHROPIC_AUTH_TOKEN during
config loading).

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ebd1e1 and 3a4234a.

⛔ Files ignored due to path filters (1)

• Cargo.toml is excluded by !**/*.toml

📒 Files selected for processing (1)

• src/config.rs