mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-30 15:03:57 +00:00
fix(medulla): resolve the backend URL like every other hosted call (#5245)
This commit is contained in:
@@ -162,7 +162,6 @@ impl Medulla<'_> {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::openhuman::medulla::all_medulla_registered_controllers;
|
||||
|
||||
/// Every method this facade dispatches must name a registered controller.
|
||||
|
||||
@@ -234,11 +234,44 @@ fn call<T>(result: Result<T, ClientError>, method: &'static str) -> Result<T, St
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// RAII guard to snapshot and restore a process-global environment variable.
|
||||
struct EnvGuard {
|
||||
key: &'static str,
|
||||
prev: Option<String>,
|
||||
}
|
||||
|
||||
impl EnvGuard {
|
||||
fn remove(key: &'static str) -> Self {
|
||||
let prev = std::env::var(key).ok();
|
||||
// SAFETY: caller holds ENV_LOCK guard.
|
||||
unsafe { std::env::remove_var(key) };
|
||||
Self { key, prev }
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for EnvGuard {
|
||||
fn drop(&mut self) {
|
||||
match &self.prev {
|
||||
// SAFETY: caller's ENV_LOCK guard is still alive during drop.
|
||||
Some(v) => unsafe { std::env::set_var(self.key, v) },
|
||||
None => unsafe { std::env::remove_var(self.key) },
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Serialize env mutation: tests modify process-globals.
|
||||
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
||||
|
||||
#[test]
|
||||
fn not_configured_encodes_an_expected_user_state_envelope() {
|
||||
let config = Config::default();
|
||||
// No api_url and no env override, so resolution fails on base URL.
|
||||
std::env::remove_var(resolve::MEDULLA_BASE_URL_ENV);
|
||||
// Isolate environment to ensure BACKEND_URL and VITE_BACKEND_URL do not
|
||||
// provide a fallback through effective_backend_api_url.
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let _env_medulla = EnvGuard::remove(resolve::MEDULLA_BASE_URL_ENV);
|
||||
let _env_backend = EnvGuard::remove("BACKEND_URL");
|
||||
let _env_vite = EnvGuard::remove("VITE_BACKEND_URL");
|
||||
let err = resolved(&config).expect_err("must not resolve");
|
||||
let decoded = StructuredRpcError::decode(&err).expect("structured envelope");
|
||||
assert!(
|
||||
@@ -301,7 +334,12 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn status_reports_unconfigured_rather_than_failing() {
|
||||
std::env::remove_var(resolve::MEDULLA_BASE_URL_ENV);
|
||||
// Isolate environment to ensure BACKEND_URL and VITE_BACKEND_URL do not
|
||||
// provide a fallback through effective_backend_api_url.
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let _env_medulla = EnvGuard::remove(resolve::MEDULLA_BASE_URL_ENV);
|
||||
let _env_backend = EnvGuard::remove("BACKEND_URL");
|
||||
let _env_vite = EnvGuard::remove("VITE_BACKEND_URL");
|
||||
let out = status(&Config::default())
|
||||
.await
|
||||
.expect("status never errors");
|
||||
|
||||
@@ -11,8 +11,14 @@
|
||||
//!
|
||||
//! `OPENHUMAN_MEDULLA_BASE_URL` remains as an override for pointing a
|
||||
//! development host at a different Medulla deployment than its OpenHuman one.
|
||||
//! Unset — the normal case — everything resolves from `api_url`.
|
||||
//! Unset — the normal case — everything resolves through
|
||||
//! [`effective_backend_api_url`], the same chain every other hosted-backend
|
||||
//! call uses: `api_url`, then the `BACKEND_URL` keys, then the prod or staging
|
||||
//! default selected by `OPENHUMAN_APP_ENV`. Resolving from the raw `api_url`
|
||||
//! instead would leave Medulla as the one backend surface with no default,
|
||||
//! reporting itself unconfigured on an install where every other call works.
|
||||
|
||||
use crate::api::config::effective_backend_api_url;
|
||||
use crate::openhuman::config::Config;
|
||||
use crate::openhuman::credentials::session_support::get_session_token;
|
||||
|
||||
@@ -55,15 +61,40 @@ impl NotConfigured {
|
||||
|
||||
/// The configured base URL, if any.
|
||||
///
|
||||
/// Precedence: `OPENHUMAN_MEDULLA_BASE_URL`, then `config.api_url`. Empty or
|
||||
/// whitespace-only values count as unset, so an exported-but-blank env var does
|
||||
/// not shadow a working config value.
|
||||
/// Precedence: `OPENHUMAN_MEDULLA_BASE_URL`, then whatever every other
|
||||
/// hosted-backend call resolves to — [`effective_backend_api_url`], which
|
||||
/// applies `config.api_url`, the `BACKEND_URL` env/compile-time keys, and
|
||||
/// finally the environment-aware default (prod or staging by
|
||||
/// `OPENHUMAN_APP_ENV`).
|
||||
///
|
||||
/// Reading `config.api_url` directly, as this used to, made Medulla the one
|
||||
/// hosted-backend surface with no default: an install that had never written an
|
||||
/// explicit `api_url` — the normal case, since every other call falls through
|
||||
/// to the default — reported "no Medulla backend configured" while auth,
|
||||
/// billing and integrations all worked. Same deployment, so it resolves the
|
||||
/// same way. It also inherits the local-AI guard for free: a user whose
|
||||
/// `api_url` points at Ollama gets the hosted backend here rather than a
|
||||
/// Medulla client aimed at a model runner.
|
||||
///
|
||||
/// Empty or whitespace-only values count as unset, so an exported-but-blank env
|
||||
/// var does not shadow a working config value.
|
||||
pub fn base_url(config: &Config) -> Option<String> {
|
||||
let from_env = std::env::var(MEDULLA_BASE_URL_ENV)
|
||||
.ok()
|
||||
.filter(|v| !v.trim().is_empty());
|
||||
|
||||
let source_ident = if from_env.is_some() {
|
||||
"env_override"
|
||||
} else {
|
||||
"effective_backend_api_url"
|
||||
};
|
||||
log::debug!(
|
||||
"[medulla] base_url source={} (URL redacted for security)",
|
||||
source_ident
|
||||
);
|
||||
|
||||
from_env
|
||||
.or_else(|| config.api_url.clone())
|
||||
.or_else(|| Some(effective_backend_api_url(&config.api_url)))
|
||||
.map(|v| v.trim().trim_end_matches('/').to_string())
|
||||
.filter(|v| !v.is_empty())
|
||||
}
|
||||
@@ -99,6 +130,38 @@ mod tests {
|
||||
/// Serialize env mutation: `base_url` reads a process-global.
|
||||
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
||||
|
||||
/// RAII guard to snapshot and restore a process-global environment variable.
|
||||
struct EnvGuard {
|
||||
key: &'static str,
|
||||
prev: Option<String>,
|
||||
}
|
||||
|
||||
impl EnvGuard {
|
||||
fn set(key: &'static str, val: &str) -> Self {
|
||||
let prev = std::env::var(key).ok();
|
||||
// SAFETY: caller holds ENV_LOCK guard.
|
||||
unsafe { std::env::set_var(key, val) };
|
||||
Self { key, prev }
|
||||
}
|
||||
|
||||
fn remove(key: &'static str) -> Self {
|
||||
let prev = std::env::var(key).ok();
|
||||
// SAFETY: caller holds ENV_LOCK guard.
|
||||
unsafe { std::env::remove_var(key) };
|
||||
Self { key, prev }
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for EnvGuard {
|
||||
fn drop(&mut self) {
|
||||
match &self.prev {
|
||||
// SAFETY: caller's ENV_LOCK guard is still alive during drop.
|
||||
Some(v) => unsafe { std::env::set_var(self.key, v) },
|
||||
None => unsafe { std::env::remove_var(self.key) },
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn config_with_api_url(url: Option<&str>) -> Config {
|
||||
let mut config = Config::default();
|
||||
config.api_url = url.map(str::to_string);
|
||||
@@ -108,16 +171,15 @@ mod tests {
|
||||
#[test]
|
||||
fn env_override_wins_over_api_url() {
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
std::env::set_var(MEDULLA_BASE_URL_ENV, "https://medulla.example");
|
||||
let _env = EnvGuard::set(MEDULLA_BASE_URL_ENV, "https://medulla.example");
|
||||
let resolved = base_url(&config_with_api_url(Some("https://api.example")));
|
||||
std::env::remove_var(MEDULLA_BASE_URL_ENV);
|
||||
assert_eq!(resolved.as_deref(), Some("https://medulla.example"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn falls_back_to_api_url_when_env_unset() {
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
std::env::remove_var(MEDULLA_BASE_URL_ENV);
|
||||
let _env = EnvGuard::remove(MEDULLA_BASE_URL_ENV);
|
||||
let resolved = base_url(&config_with_api_url(Some("https://api.example/")));
|
||||
assert_eq!(resolved.as_deref(), Some("https://api.example"));
|
||||
}
|
||||
@@ -127,25 +189,50 @@ mod tests {
|
||||
// An exported-but-empty var is a common shell accident; treating it as
|
||||
// "configured" would break an otherwise-working setup.
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
std::env::set_var(MEDULLA_BASE_URL_ENV, " ");
|
||||
let _env = EnvGuard::set(MEDULLA_BASE_URL_ENV, " ");
|
||||
let resolved = base_url(&config_with_api_url(Some("https://api.example")));
|
||||
std::env::remove_var(MEDULLA_BASE_URL_ENV);
|
||||
assert_eq!(resolved.as_deref(), Some("https://api.example"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn none_when_neither_is_set() {
|
||||
fn an_unconfigured_install_falls_back_to_the_hosted_backend() {
|
||||
// Medulla and the OpenHuman backend are one deployment, so an install
|
||||
// that never wrote an explicit `api_url` — the normal case — must reach
|
||||
// the same host every other backend call defaults to, not report itself
|
||||
// unconfigured while auth and billing work.
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
std::env::remove_var(MEDULLA_BASE_URL_ENV);
|
||||
assert_eq!(base_url(&config_with_api_url(None)), None);
|
||||
let _env_medulla = EnvGuard::remove(MEDULLA_BASE_URL_ENV);
|
||||
let _env_backend = EnvGuard::remove("BACKEND_URL");
|
||||
let _env_vite = EnvGuard::remove("VITE_BACKEND_URL");
|
||||
assert_eq!(
|
||||
base_url(&config_with_api_url(None)),
|
||||
Some(crate::api::config::effective_backend_api_url(&None))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_local_model_runner_url_does_not_become_the_medulla_host() {
|
||||
// `api_url` doubles as the inference endpoint. Pointing it at Ollama
|
||||
// must not aim the Medulla client at a model runner that 404s every
|
||||
// path it speaks — the same guard every other backend call gets.
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let _env_medulla = EnvGuard::remove(MEDULLA_BASE_URL_ENV);
|
||||
let _env_backend = EnvGuard::remove("BACKEND_URL");
|
||||
let _env_vite = EnvGuard::remove("VITE_BACKEND_URL");
|
||||
let resolved = base_url(&config_with_api_url(Some("http://localhost:11434")));
|
||||
// Assert that the local model runner URL does not become the Medulla host;
|
||||
// instead, it falls back to the effective backend URL (same as all other backend calls).
|
||||
assert_eq!(
|
||||
resolved,
|
||||
Some(crate::api::config::effective_backend_api_url(&None))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trailing_slashes_are_trimmed_so_paths_do_not_double_up() {
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
std::env::set_var(MEDULLA_BASE_URL_ENV, "https://medulla.example///");
|
||||
let _env = EnvGuard::set(MEDULLA_BASE_URL_ENV, "https://medulla.example///");
|
||||
let resolved = base_url(&config_with_api_url(None));
|
||||
std::env::remove_var(MEDULLA_BASE_URL_ENV);
|
||||
assert_eq!(resolved.as_deref(), Some("https://medulla.example"));
|
||||
}
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ use std::sync::Arc;
|
||||
|
||||
use chrono::Utc;
|
||||
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
use crate::openhuman::people::address_book;
|
||||
use crate::openhuman::people::resolver::HandleResolver;
|
||||
use crate::openhuman::people::store::PeopleStore;
|
||||
|
||||
Reference in New Issue
Block a user