From 3829d8126941f08374955e1f616a875940cdab68 Mon Sep 17 00:00:00 2001
From: alexthebuildr <116134064+ztsalexey@users.noreply.github.com>
Date: Fri, 20 Feb 2026 01:11:10 -0700
Subject: [PATCH] fix: consolidate per-module ENV_MUTEX into crate-wide test
 lock (#246)

Each config test module (llm.rs, embeddings.rs) defined its own
ENV_MUTEX, which doesn't prevent cross-module env races since
cargo test runs in parallel. Move to a single shared mutex in
config/helpers.rs so all unsafe set_var/remove_var calls are
serialized crate-wide.

Closes #245

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/config/embeddings.rs | 8 ++------
 src/config/helpers.rs    | 9 +++++++++
 src/config/llm.rs        | 5 +----
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/config/embeddings.rs b/src/config/embeddings.rs
index 39f28f06..4528aded 100644
--- a/src/config/embeddings.rs
+++ b/src/config/embeddings.rs
@@ -102,16 +102,12 @@ impl EmbeddingsConfig {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::config::helpers::ENV_MUTEX;
     use crate::settings::{EmbeddingsSettings, Settings};
-    use std::sync::Mutex;
-
-    /// Serializes env-mutating tests to prevent parallel races.
-    static ENV_MUTEX: Mutex<()> = Mutex::new(());
 
     /// Clear all embedding-related env vars.
     fn clear_embedding_env() {
-        // SAFETY: Only called under ENV_MUTEX in tests. No other threads
-        // observe these vars while the lock is held.
+        // SAFETY: Only called under ENV_MUTEX in tests.
         unsafe {
             std::env::remove_var("EMBEDDING_ENABLED");
             std::env::remove_var("EMBEDDING_PROVIDER");
diff --git a/src/config/helpers.rs b/src/config/helpers.rs
index b463bb50..e9e966df 100644
--- a/src/config/helpers.rs
+++ b/src/config/helpers.rs
@@ -2,6 +2,15 @@ use crate::error::ConfigError;
 
 use super::INJECTED_VARS;
 
+/// Crate-wide mutex for tests that mutate process environment variables.
+///
+/// The process environment is global state shared across all threads.
+/// Per-module mutexes do NOT prevent races between modules running in
+/// parallel.  Every `unsafe { set_var / remove_var }` call in tests
+/// MUST hold this single lock.
+#[cfg(test)]
+pub(crate) static ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(());
+
 pub(crate) fn optional_env(key: &str) -> Result<Option<String>, ConfigError> {
     // Check real env vars first (always win over injected secrets)
     match std::env::var(key) {
diff --git a/src/config/llm.rs b/src/config/llm.rs
index a7564011..947b6da1 100644
--- a/src/config/llm.rs
+++ b/src/config/llm.rs
@@ -383,11 +383,8 @@ fn default_session_path() -> PathBuf {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::config::helpers::ENV_MUTEX;
     use crate::settings::Settings;
-    use std::sync::Mutex;
-
-    /// Serializes env-mutating tests to prevent parallel races.
-    static ENV_MUTEX: Mutex<()> = Mutex::new(());
 
     /// Clear all openai-compatible-related env vars.
     fn clear_openai_compatible_env() {