test: isolate config-path readers from env redirects

[Hmbown]

Summary

Fixes the test-environment reader race behind the intermittent Windows provider-persistence failures in #4463.

• serialize test-only CODEWHALE_CONFIG_PATH and DEEPSEEK_CONFIG_PATH readers with the existing process-wide environment lock
• preserve same-thread reentrant reads for tests that own a temporary redirect
• make EnvVarGuard assert that callers actually hold the lock
• add a cross-thread regression proving a reader cannot observe another test temporary path

Production config-path behavior is unchanged; the synchronization is cfg(test) only.

Closes #4463

Verification

• full codewhale-tui suite: 7,111 passed, 0 failed, 2 ignored
• both historical provider-persistence flake tests passed
• new regression fails before this fix and passes after it
• cargo check -p codewhale-tui
• cargo check -p codewhale-tui --tests
• formatting and diff checks

The remaining acceptance gate is native windows-latest CI. Local cross-compilation on macOS stopped in ring because the Windows SDK header assert.h is unavailable, before Codewhale code compiled.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @Hmbown's task in 4m 45s —— View job

---

Review: test-isolate config-path readers from env redirects

• Gather context (read diff, changed files)
• Understand the request (code review per custom instructions)
• Review correctness / race conditions in test sync changes
• Review provider/model route safety implications (N/A — no such code touched)
• Review reuse/simplification
• Review test coverage
• Review security
• Post final review

Summary

This is a well-scoped, cfg(test)-only fix. I traced the new owner/lock protocol in crates/tui/src/test_support.rs:1-105 by hand and it holds up:

• TestEnvLock's custom Drop clears the thread-owner marker (crates/tui/src/test_support.rs:36-46) before Rust auto-drops the wrapped MutexGuard field (fields drop after a type's own Drop::drop body finishes) — this is exactly what's needed to prevent the "stale owner during a new holder's registration window" race the doc comment describes at test_support.rs:24-29. I walked through the interleavings (foreign reader vs. writer, writer-drops-during-registration-of-next-writer) and didn't find a gap.
• with_test_env_lock (test_support.rs:65-80) correctly distinguishes "I already hold the lock" (same-thread reentrant read, no blocking) from "someone else holds it" (block on the real Mutex), using try_lock + the owner marker.
• The two production reader call sites that actually needed this — config/paths.rs:86-95 (env_config_path) and settings.rs:1394-1412 (legacy_config_override_parent, shared by TuiPrefs::path and settings_path_candidates) — are now both wrapped. I grepped the whole crate for other direct reads of DEEPSEEK_CONFIG_PATH/CODEWHALE_CONFIG_PATH outside these two functions and confirmed every other read site is a test-only writer guard that already holds lock_test_env() for its whole lifetime, so this covers the full production reader surface.
• The new regression test (test_support.rs:143-181) is a good, non-flaky proof: it uses an mpsc channel + recv_timeout to assert a background reader thread is still blocked while the writer holds the redirect, then confirms it resolves only after the guard drops.
• The EnvVarGuard::set/remove debug_assert! additions (test_support.rs:117-132) are a nice belt-and-suspenders check that will catch any future caller who mutates env vars without holding the lock.
• The 15 mechanical std::sync::MutexGuard<'static, ()> → crate::test_support::TestEnvLock renames across command/tui/snapshot test modules are pure type-signature updates with no behavior change — verified each diff, all consistent.

Design note (not a bug, worth a short comment somewhere): because TuiPrefs::path()/settings_path_candidates()/env_config_path() are now blocking calls in test builds, any future test that holds lock_test_env()/an EnvVarGuard while driving a #[tokio::test(flavor = "multi_thread", ...)] task that calls into one of these functions from a different OS thread than the lock holder would deadlock instead of race (ownership tracking is per-ThreadId, so it isn't recognized as reentrant across threads). I checked the current suite for this shape (multi-thread tokio tests combined with lock_test_env) and didn't find one today, so this isn't an active bug — just a sharp edge for the next contributor who reaches for these guards inside async multi-threaded tests.

Production (non-test) behavior is unchanged — both #[cfg(test)]-gated call sites fall through to the identical unlocked read path in release builds, and test_support itself is only compiled under #[cfg(test)] (main.rs:116-117).

No correctness bugs, no provider/model-routing concerns (none of this touches that code), no security issues, and test coverage for the new behavior is solid.

I wasn't able to execute cargo check/cargo test directly in this sandbox (build commands require approval that isn't available in this automated context), so this review is based on a full manual trace of the diff plus the PR's own reported local verification (7,111 passed, 0 failed). CI on this PR is currently still in progress (CI and Claude PR Review workflows); Contribution gate, DCO, and PR #4483 checks have already passed. The Windows-specific acceptance the PR is targeting will only be confirmed by native windows-latest CI as the author notes.