fix(skills): thread local-http install flag as param, not process env (#4567) (#4584)

docs/README.de.md

@@ -43,7 +43,7 @@
 <p align="center">
  <img src="https://img.shields.io/badge/status-early%20beta-orange" alt="Frühe Beta" />
  <a href="https://github.com/tinyhumansai/openhuman/releases/latest"><img src="https://img.shields.io/github/v/release/tinyhumansai/openhuman?label=latest" alt="Aktuellste Version" /></a>
- <a href="https://github.com/tinyhumansai/openhuman/stargazers"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="GitHub Stars" /></a>
+ <a href="https://github.com/tinyhumansai/openhuman"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="GitHub Stars" /></a>
  <a href="../LICENSE"><img src="https://img.shields.io/github/license/tinyhumansai/openhuman" alt="Lizenz" /></a>
 </p>
 

docs/README.ja-JP.md

@@ -43,7 +43,7 @@
 <p align="center">
  <img src="https://img.shields.io/badge/status-early%20beta-orange" alt="Early Beta" />
  <a href="https://github.com/tinyhumansai/openhuman/releases/latest"><img src="https://img.shields.io/github/v/release/tinyhumansai/openhuman?label=latest" alt="最新リリース" /></a>
- <a href="https://github.com/tinyhumansai/openhuman/stargazers"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="GitHub Stars" /></a>
+ <a href="https://github.com/tinyhumansai/openhuman"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="GitHub Stars" /></a>
  <a href="../LICENSE"><img src="https://img.shields.io/github/license/tinyhumansai/openhuman" alt="ライセンス" /></a>
 </p>
 

docs/README.ko.md

@@ -43,7 +43,7 @@
 <p align="center">
  <img src="https://img.shields.io/badge/status-early%20beta-orange" alt="얼리 베타" />
  <a href="https://github.com/tinyhumansai/openhuman/releases/latest"><img src="https://img.shields.io/github/v/release/tinyhumansai/openhuman?label=latest" alt="최신 릴리스" /></a>
- <a href="https://github.com/tinyhumansai/openhuman/stargazers"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="GitHub Stars" /></a>
+ <a href="https://github.com/tinyhumansai/openhuman"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="GitHub Stars" /></a>
  <a href="../LICENSE"><img src="https://img.shields.io/github/license/tinyhumansai/openhuman" alt="라이선스" /></a>
 </p>
 

docs/README.ur-pk.md

@@ -53,7 +53,7 @@
 <p align="center">
  <img src="https://img.shields.io/badge/status-early%20beta-orange" alt="ابتدائی آزمائشی نسخہ" />
  <a href="https://github.com/tinyhumansai/openhuman/releases/latest"><img src="https://img.shields.io/github/v/release/tinyhumansai/openhuman?label=latest" alt="تازہ ترین نسخہ" /></a>
- <a href="https://github.com/tinyhumansai/openhuman/stargazers"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="ستارے" /></a>
+ <a href="https://github.com/tinyhumansai/openhuman"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="ستارے" /></a>
  <a href="../LICENSE"><img src="https://img.shields.io/github/license/tinyhumansai/openhuman" alt="لائسنس" /></a>
 </p>
 

docs/README.zh-CN.md

@@ -43,7 +43,7 @@
 <p align="center">
  <img src="https://img.shields.io/badge/status-early%20beta-orange" alt="早期测试版" />
  <a href="https://github.com/tinyhumansai/openhuman/releases/latest"><img src="https://img.shields.io/github/v/release/tinyhumansai/openhuman?label=latest" alt="最新版本" /></a>
- <a href="https://github.com/tinyhumansai/openhuman/stargazers"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="GitHub Stars" /></a>
+ <a href="https://github.com/tinyhumansai/openhuman"><img src="https://img.shields.io/github/stars/tinyhumansai/openhuman?style=flat" alt="GitHub Stars" /></a>
  <a href="../LICENSE"><img src="https://img.shields.io/github/license/tinyhumansai/openhuman" alt="许可证" /></a>
 </p>
 

src/openhuman/skills/ops_install.rs

@@ -119,7 +119,9 @@ pub async fn install_workflow_from_url(
     params: InstallWorkflowFromUrlParams,
 ) -> Result<InstallWorkflowFromUrlOutcome, String> {
     let home = dirs::home_dir();
-    install_workflow_from_url_with_home(workspace_dir, params, home.as_deref()).await
+    let allow_local_http = read_allow_local_http_env();
+    install_workflow_from_url_with_home(workspace_dir, params, home.as_deref(), allow_local_http)
+        .await
 }
 
 pub(crate) fn should_report_install_fetch_status(status: reqwest::StatusCode) -> bool {
@@ -130,9 +132,10 @@ pub(crate) async fn install_workflow_from_url_with_home(
     workspace_dir: &Path,
     params: InstallWorkflowFromUrlParams,
     home: Option<&Path>,
+    allow_local_http: bool,
 ) -> Result<InstallWorkflowFromUrlOutcome, String> {
     let raw_url = params.url.trim().to_string();
-    validate_install_url(&raw_url)?;
+    validate_install_url_with_config(&raw_url, allow_local_http)?;
 
     let timeout_secs = params
         .timeout_secs
@@ -148,7 +151,7 @@ pub(crate) async fn install_workflow_from_url_with_home(
     // resolver may see different answers than ours. Closing that gap requires
     // pinning a `SocketAddr` and passing it to reqwest via a custom resolver,
     // tracked separately.
-    if !allow_local_http_install_url(&fetch_url) {
+    if !(allow_local_http && is_loopback_http_url(&fetch_url)) {
         validate_resolved_host(&fetch_url).await?;
     }
 
@@ -698,6 +701,19 @@ pub(crate) fn derive_install_slug(fm: &WorkflowFrontmatter) -> Result<String, St
 /// * IPv6 literals in loopback (::1), unspecified (::), unique-local
 ///   (fc00::/7), link-local (fe80::/10), or multicast (ff00::/8)
 pub fn validate_install_url(raw: &str) -> Result<(), String> {
+    validate_install_url_with_config(raw, read_allow_local_http_env())
+}
+
+/// Same as [`validate_install_url`] but takes an explicit `allow_local_http`
+/// flag instead of reading `OPENHUMAN_SKILL_INSTALL_ALLOW_LOCAL_HTTP` from the
+/// process environment. Callers below the public entry point should always use
+/// this variant — the env-var read in `validate_install_url` is racy across
+/// threads under the parallel test runner (#4567), and the escape hatch is
+/// only meaningful at a single boundary.
+pub(crate) fn validate_install_url_with_config(
+    raw: &str,
+    allow_local_http: bool,
+) -> Result<(), String> {
     let trimmed = raw.trim();
     if trimmed.is_empty() {
         return Err("url must not be empty".to_string());
@@ -710,7 +726,7 @@ pub fn validate_install_url(raw: &str) -> Result<(), String> {
     }
     let parsed = url::Url::parse(trimmed).map_err(|e| format!("invalid url {trimmed:?}: {e}"))?;
     if parsed.scheme() != "https" {
-        if allow_local_http_install_url(trimmed) {
+        if allow_local_http && is_loopback_http_url(trimmed) {
             return Ok(());
         }
         return Err(format!(
@@ -732,10 +748,15 @@ pub fn validate_install_url(raw: &str) -> Result<(), String> {
     Ok(())
 }
 
-fn allow_local_http_install_url(raw: &str) -> bool {
-    if std::env::var(ALLOW_LOCAL_HTTP_ENV).ok().as_deref() != Some("1") {
-        return false;
-    }
+/// Reads the local-HTTP install escape hatch env var. Kept as the single
+/// boundary point where env is inspected so downstream validation never
+/// touches process-global state — required to stop the parallel-test race
+/// described in #4567.
+fn read_allow_local_http_env() -> bool {
+    std::env::var(ALLOW_LOCAL_HTTP_ENV).ok().as_deref() == Some("1")
+}
+
+fn is_loopback_http_url(raw: &str) -> bool {
     let Ok(parsed) = url::Url::parse(raw) else {
         return false;
     };
@@ -907,15 +928,12 @@ mod install_fetch_tests {
     /// for both a 4xx (which is NOT reported to Sentry) and a 5xx (which is).
     /// Exercises both branches of the non-2xx handling; the report-suppression
     /// polarity itself is asserted by `is_skills_install_client_error_event` in
-    /// observability. Uses the local-HTTP install escape hatch so the loopback
-    /// mock passes URL validation.
+    /// observability. Passes the local-HTTP install escape hatch as an
+    /// explicit param so the loopback mock passes URL validation — the env-var
+    /// path is process-global and races with other env-touching tests under
+    /// parallel execution (#4567).
     #[tokio::test]
     async fn non_2xx_install_fetch_returns_err_for_4xx_and_5xx() {
-        let _env = crate::openhuman::config::TEST_ENV_LOCK
-            .lock()
-            .unwrap_or_else(|e| e.into_inner());
-        std::env::set_var(ALLOW_LOCAL_HTTP_ENV, "1");
-
         let tmp = TempDir::new().unwrap();
 
         for status in [StatusCode::NOT_FOUND, StatusCode::INTERNAL_SERVER_ERROR] {
@@ -928,6 +946,7 @@ mod install_fetch_tests {
                     timeout_secs: Some(5),
                 },
                 None,
+                true,
             )
             .await
             .expect_err("a non-2xx fetch must return Err so the UI surfaces it");
@@ -936,7 +955,5 @@ mod install_fetch_tests {
                 "error must surface the status to the UI: {err}"
             );
         }
-
-        std::env::remove_var(ALLOW_LOCAL_HTTP_ENV);
     }
 }

src/openhuman/skills/ops_tests.rs

@@ -7,30 +7,6 @@ fn write(path: &Path, content: &str) {
     std::fs::write(path, content).unwrap();
 }
 
-struct EnvVarGuard {
-    key: &'static str,
-    previous: Option<std::ffi::OsString>,
-}
-
-impl EnvVarGuard {
-    fn set(key: &'static str, value: &str) -> Self {
-        let previous = std::env::var_os(key);
-        unsafe { std::env::set_var(key, value) };
-        Self { key, previous }
-    }
-}
-
-impl Drop for EnvVarGuard {
-    fn drop(&mut self) {
-        unsafe {
-            match &self.previous {
-                Some(value) => std::env::set_var(self.key, value),
-                None => std::env::remove_var(self.key),
-            }
-        }
-    }
-}
-
 /// Workspace-only variant of [`load_workflow_metadata`] used by tests that care only
 /// about project-scope semantics. The production [`load_workflow_metadata`] now
 /// consults `dirs::home_dir()`; in unit tests that would non-deterministically
@@ -1214,7 +1190,6 @@ async fn install_workflow_from_url_is_idempotent_when_skill_already_exists() {
     use wiremock::matchers::{method, path};
     use wiremock::{Mock, MockServer, ResponseTemplate};
 
-    let _guard = EnvVarGuard::set("OPENHUMAN_SKILL_INSTALL_ALLOW_LOCAL_HTTP", "1");
     let server = MockServer::start().await;
     Mock::given(method("GET"))
         .and(path("/SKILL.md"))
@@ -1231,15 +1206,23 @@ async fn install_workflow_from_url_is_idempotent_when_skill_already_exists() {
         timeout_secs: Some(5),
     };
 
-    let first =
-        install_workflow_from_url_with_home(workspace.path(), params.clone(), Some(home.path()))
-            .await
-            .unwrap();
+    // Pass the local-HTTP escape hatch as an explicit param — the env-var
+    // path is process-global and races other env-touching tests under
+    // parallel execution (#4567).
+    let first = install_workflow_from_url_with_home(
+        workspace.path(),
+        params.clone(),
+        Some(home.path()),
+        true,
+    )
+    .await
+    .unwrap();
     assert_eq!(first.new_skills, vec!["apple-notes"]);
 
-    let second = install_workflow_from_url_with_home(workspace.path(), params, Some(home.path()))
-        .await
-        .unwrap();
+    let second =
+        install_workflow_from_url_with_home(workspace.path(), params, Some(home.path()), true)
+            .await
+            .unwrap();
     assert!(second.new_skills.is_empty(), "{second:?}");
     assert!(second.stdout.contains("already installed"), "{second:?}");
 }