fix(tests): stop TEST_ENV_LOCK poison cascade turning 1 panic into 38 (tinyhumansai#1604)

Author: senamakel

src/core/jsonrpc_tests.rs

@@ -735,10 +735,19 @@ async fn thread_not_found_rpc_error_does_not_report_to_sentry() {
     let _sentry_guard = sentry::HubSwitchGuard::new(sentry_hub);
 
     let subscriber = tracing_subscriber::registry().with(
-        sentry::integrations::tracing::layer().event_filter(|metadata| match *metadata.level() {
-            Level::ERROR => sentry::integrations::tracing::EventFilter::Event,
-            Level::WARN | Level::INFO => sentry::integrations::tracing::EventFilter::Breadcrumb,
-            _ => sentry::integrations::tracing::EventFilter::Ignore,
+        sentry::integrations::tracing::layer().event_filter(|metadata| {
+            // Mirror the production sentry-tracing layer: events emitted from
+            // `report_error_message` are captured directly via
+            // `sentry::capture_message` and must not be picked up here too
+            // (otherwise this test sees double events).
+            if metadata.target() == crate::core::observability::REPORT_ERROR_TRACING_TARGET {
+                return sentry::integrations::tracing::EventFilter::Ignore;
+            }
+            match *metadata.level() {
+                Level::ERROR => sentry::integrations::tracing::EventFilter::Event,
+                Level::WARN | Level::INFO => sentry::integrations::tracing::EventFilter::Breadcrumb,
+                _ => sentry::integrations::tracing::EventFilter::Ignore,
+            }
         }),
     );
     let _subscriber_guard = tracing::subscriber::set_default(subscriber);

src/core/logging.rs

@@ -354,6 +354,13 @@ where
     S: tracing::Subscriber + for<'a> LookupSpan<'a>,
 {
     sentry::integrations::tracing::layer().event_filter(|md: &tracing::Metadata<'_>| {
+        // Events emitted from `report_error_message` are captured directly via
+        // `sentry::capture_message` at the call site (see
+        // `core::observability::REPORT_ERROR_TRACING_TARGET` for rationale).
+        // Skip them here so we don't double-report.
+        if md.target() == crate::core::observability::REPORT_ERROR_TRACING_TARGET {
+            return sentry::integrations::tracing::EventFilter::Ignore;
+        }
         match *md.level() {
             Level::ERROR => sentry::integrations::tracing::EventFilter::Event,
             Level::WARN | Level::INFO => sentry::integrations::tracing::EventFilter::Breadcrumb,
@@ -372,7 +379,7 @@ mod tests {
     static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
 
     fn with_clean_rust_log<R>(f: impl FnOnce() -> R) -> R {
-        let _guard = ENV_LOCK.lock().unwrap();
+        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let prior = std::env::var("RUST_LOG").ok();
         std::env::remove_var("RUST_LOG");
         let result = f();
@@ -435,7 +442,7 @@ mod tests {
 
     #[test]
     fn seed_rust_log_respects_existing_value() {
-        let _guard = ENV_LOCK.lock().unwrap();
+        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let prior = std::env::var("RUST_LOG").ok();
         std::env::set_var("RUST_LOG", "warn");
         seed_rust_log(true, CliLogDefault::Global);
@@ -456,7 +463,7 @@ mod tests {
 
     #[test]
     fn parse_log_file_constraints_handles_csv_and_whitespace() {
-        let _guard = ENV_LOCK.lock().unwrap();
+        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let prior = std::env::var("OPENHUMAN_LOG_FILE_CONSTRAINTS").ok();
         std::env::set_var("OPENHUMAN_LOG_FILE_CONSTRAINTS", "rpc, , agent ,memory");
         let parsed = parse_log_file_constraints();

src/core/observability.rs

@@ -119,6 +119,27 @@ fn report_expected_message(kind: ExpectedErrorKind, message: &str, domain: &str,
     }
 }
 
+/// Distinct `tracing::Metadata::target()` we set on the diagnostic
+/// `tracing::error!` emitted from [`report_error_message`].
+///
+/// Sentry capture for this helper happens via an explicit
+/// `sentry::capture_message` call below — not via the `sentry-tracing`
+/// layer scooping up the `tracing::error!` event. The production
+/// `sentry_tracing_layer()` in `core::logging` filters events with this
+/// target to `EventFilter::Ignore` so we never double-report (one direct
+/// `capture_message`, one tracing-bridge capture of the same condition).
+///
+/// Why direct capture instead of relying on the bridge: the bridge worked
+/// in steady-state but flaked under parallel test scheduling
+/// (`thread_not_found_rpc_error_does_not_report_to_sentry` repeatedly hit
+/// `events.len() == 0` in CI even with a thread-default subscriber wired
+/// up — likely a Linux-only thread-local ordering quirk in
+/// `sentry-tracing`'s `Hub::current()` lookup at event-emit time). Direct
+/// `sentry::capture_message` synchronously routes through the active hub
+/// and is deterministic, which keeps both production reporting and tests
+/// honest.
+pub const REPORT_ERROR_TRACING_TARGET: &str = "openhuman::observability::report_error";
+
 pub(crate) fn report_error_message(
     message: &str,
     domain: &str,
@@ -134,7 +155,15 @@ pub(crate) fn report_error_message(
             }
         },
         || {
+            // Direct, synchronous Sentry capture — see
+            // `REPORT_ERROR_TRACING_TARGET` for why we don't rely on the
+            // `sentry-tracing` layer for this call site.
+            sentry::capture_message(message, sentry::Level::Error);
+            // Diagnostic log line for stderr / file appenders. Tagged with
+            // the marker target so the production sentry-tracing layer
+            // skips it (no double Sentry event).
             tracing::error!(
+                target: REPORT_ERROR_TRACING_TARGET,
                 domain = domain,
                 operation = operation,
                 error = %message,

src/openhuman/composio/periodic.rs

@@ -285,7 +285,7 @@ mod tests {
     async fn run_one_tick_returns_ok_when_no_client() {
         // Isolate the workspace/env so config loading doesn't contend with
         // sibling tests mutating OPENHUMAN_WORKSPACE in parallel.
-        let _guard = ENV_LOCK.lock().expect("env lock");
+        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let tmp = tempdir().expect("tempdir");
         unsafe {
             std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());

src/openhuman/config/ops_tests.rs

@@ -37,7 +37,7 @@ use crate::openhuman::config::TEST_ENV_LOCK as ENV_LOCK;
 
 #[test]
 fn env_flag_enabled_recognizes_truthy_forms() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let key = "OPENHUMAN_TEST_FLAG_A";
     for truthy in ["1", "true", "TRUE", "yes", "YES"] {
         unsafe {
@@ -61,7 +61,7 @@ fn env_flag_enabled_recognizes_truthy_forms() {
 
 #[test]
 fn core_rpc_url_from_env_returns_default_when_unset() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     unsafe {
         std::env::remove_var("OPENHUMAN_CORE_RPC_URL");
     }
@@ -70,7 +70,7 @@ fn core_rpc_url_from_env_returns_default_when_unset() {
 
 #[test]
 fn core_rpc_url_from_env_uses_override_when_set() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     unsafe {
         std::env::set_var("OPENHUMAN_CORE_RPC_URL", "http://1.2.3.4:9999/rpc");
     }
@@ -116,7 +116,7 @@ fn config_openhuman_dir_returns_config_path_parent() {
 
 #[test]
 fn get_runtime_flags_reads_env_overrides() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     unsafe {
         std::env::remove_var("OPENHUMAN_BROWSER_ALLOW_ALL");
     }
@@ -128,7 +128,7 @@ fn get_runtime_flags_reads_env_overrides() {
 
 #[test]
 fn set_browser_allow_all_toggles_env_var() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let before = std::env::var("OPENHUMAN_BROWSER_ALLOW_ALL").ok();
 
     let _ = set_browser_allow_all(true);
@@ -503,7 +503,7 @@ async fn get_config_snapshot_wraps_snapshot_in_rpc_outcome() {
 
 #[tokio::test]
 async fn load_and_apply_dictation_settings_rejects_invalid_activation_mode() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -525,7 +525,7 @@ async fn load_and_apply_dictation_settings_rejects_invalid_activation_mode() {
 
 #[tokio::test]
 async fn load_and_apply_voice_server_settings_rejects_invalid_activation_mode() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -550,7 +550,7 @@ async fn load_and_apply_voice_server_settings_rejects_invalid_activation_mode()
 
 #[tokio::test]
 async fn load_and_apply_dictation_settings_accepts_valid_modes() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -576,7 +576,7 @@ async fn load_and_apply_dictation_settings_accepts_valid_modes() {
 
 #[tokio::test]
 async fn load_and_apply_voice_server_settings_accepts_valid_modes_and_clamps() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -609,7 +609,7 @@ async fn load_and_apply_voice_server_settings_accepts_valid_modes_and_clamps() {
 
 #[tokio::test]
 async fn get_dictation_settings_reads_from_loaded_config() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -625,7 +625,7 @@ async fn get_dictation_settings_reads_from_loaded_config() {
 
 #[tokio::test]
 async fn get_voice_server_settings_reads_from_loaded_config() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -640,7 +640,7 @@ async fn get_voice_server_settings_reads_from_loaded_config() {
 
 #[tokio::test]
 async fn get_onboarding_completed_reads_from_loaded_config() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -655,7 +655,7 @@ async fn get_onboarding_completed_reads_from_loaded_config() {
 
 #[tokio::test]
 async fn load_and_resolve_api_url_returns_api_url_in_response() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -669,7 +669,7 @@ async fn load_and_resolve_api_url_returns_api_url_in_response() {
 
 #[tokio::test]
 async fn workspace_onboarding_flag_resolve_rejects_invalid_and_defaults() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -691,7 +691,7 @@ async fn workspace_onboarding_flag_resolve_rejects_invalid_and_defaults() {
 
 #[tokio::test]
 async fn workspace_onboarding_flag_set_rejects_invalid_names() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());
@@ -709,7 +709,7 @@ async fn workspace_onboarding_flag_set_rejects_invalid_names() {
 
 #[tokio::test]
 async fn workspace_onboarding_flag_set_round_trip() {
-    let _g = ENV_LOCK.lock().unwrap();
+    let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
     let tmp = tempdir().unwrap();
     unsafe {
         std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path());

src/openhuman/credentials/cli.rs

@@ -181,7 +181,7 @@ mod tests {
 
     #[tokio::test]
     async fn cli_auth_login_provider_branch_stores_credentials() {
-        let _g = ENV_LOCK.lock().unwrap();
+        let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let tmp = TempDir::new().unwrap();
         set_workspace(&tmp);
         let result = cli_auth_login(
@@ -204,7 +204,7 @@ mod tests {
 
     #[tokio::test]
     async fn cli_auth_login_with_non_empty_fields_passes_them_through() {
-        let _g = ENV_LOCK.lock().unwrap();
+        let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let tmp = TempDir::new().unwrap();
         set_workspace(&tmp);
         let fields = serde_json::json!({ "org_id": "org-1" });
@@ -216,7 +216,7 @@ mod tests {
 
     #[tokio::test]
     async fn cli_auth_logout_provider_branch_reports_no_op_on_empty_store() {
-        let _g = ENV_LOCK.lock().unwrap();
+        let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let tmp = TempDir::new().unwrap();
         set_workspace(&tmp);
         let result = cli_auth_logout("openai".into(), None).await;
@@ -230,7 +230,7 @@ mod tests {
 
     #[tokio::test]
     async fn cli_auth_status_provider_branch_lists_for_provider() {
-        let _g = ENV_LOCK.lock().unwrap();
+        let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let tmp = TempDir::new().unwrap();
         set_workspace(&tmp);
         let result = cli_auth_status("openai".into(), None).await;
@@ -245,7 +245,7 @@ mod tests {
 
     #[tokio::test]
     async fn cli_auth_list_with_empty_filter_lists_all() {
-        let _g = ENV_LOCK.lock().unwrap();
+        let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let tmp = TempDir::new().unwrap();
         set_workspace(&tmp);
         let out = cli_auth_list(None).await.expect("list ok");
@@ -256,7 +256,7 @@ mod tests {
 
     #[tokio::test]
     async fn cli_auth_list_rejects_whitespace_only_filter_as_no_filter() {
-        let _g = ENV_LOCK.lock().unwrap();
+        let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let tmp = TempDir::new().unwrap();
         set_workspace(&tmp);
         let out = cli_auth_list(Some("   ".into())).await.expect("list ok");

src/openhuman/local_ai/install.rs

@@ -468,6 +468,33 @@ mod tests {
     #[test]
     fn find_system_ollama_binary_detects_macos_app_bundle_in_applications() {
         let _lock = env_lock();
+        // `find_system_ollama_binary` probes a fixed priority list on macOS:
+        //   1. /usr/local/bin/ollama   (intel homebrew, hand-installed)
+        //   2. /opt/homebrew/bin/ollama (apple-silicon homebrew)
+        //   3. /Applications/Ollama.app/Contents/Resources/ollama
+        //   4. $HOME/Applications/Ollama.app/Contents/Resources/ollama
+        // The test exercises (4) by pointing $HOME at a tempdir and clearing
+        // PATH/OLLAMA_BIN. Paths (1)–(3) are absolute and cannot be redirected
+        // — if a dev machine already has Ollama installed at either homebrew
+        // location or in the system /Applications dir, the function returns
+        // that real binary first and the assertion below fails. Skip when any
+        // earlier candidate already resolves so this test stays a regression
+        // gate on the ~/Applications branch and not a "is Ollama installed on
+        // this CI runner" probe.
+        let unmaskable_real_install = [
+            "/usr/local/bin/ollama",
+            "/opt/homebrew/bin/ollama",
+            "/Applications/Ollama.app/Contents/Resources/ollama",
+        ]
+        .iter()
+        .any(|p| std::path::Path::new(p).is_file());
+        if unmaskable_real_install {
+            eprintln!(
+                "skipping: host has a real Ollama install at a higher-priority absolute path \
+                 the test cannot mock"
+            );
+            return;
+        }
         let tmp = tempfile::tempdir().unwrap();
         // Build a fake /Applications/Ollama.app/Contents/Resources/ollama tree.
         let bundle_bin = tmp