Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/openhuman/agent/experience/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ async fn open_store_in_subdir(
let memory = crate::openhuman::memory::store::UnifiedMemory::new_with_memory_dir(
&config.workspace_dir,
memory_subdir,
crate::openhuman::inference::embeddings::default_embedding_provider(),
// Config-scoped so the experience store's managed embedder reads the
// signed-in user's session, not the keyless `default_state_dir()`
// scope (#5501).
crate::openhuman::inference::embeddings::default_embedding_provider_with_config(config),
config.memory.sqlite_open_timeout_secs,
)
.map_err(|e| format!("open agent experience store '{memory_subdir}': {e:#}"))?;
Expand Down
127 changes: 127 additions & 0 deletions src/openhuman/inference/embeddings/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,50 @@ fn managed_credential_scope(config: &Config) -> (Option<PathBuf>, bool) {
(Some(state_dir_from_config(config)), config.secrets.encrypt)
}

/// Returns the default embedding provider — cloud (OpenHuman backend, Voyage) —
/// scoped to `config`'s credential store.
///
/// This is the [`default_embedding_provider`] every caller that holds a
/// `&Config` must use. It threads the caller's real credential-store location
/// ([`managed_credential_scope`]) into the cloud embedder's bearer resolver — the
/// same `(state_dir, encrypt)` pair sign-in wrote the `app-session` token to — so
/// a signed-in user's ingest/seal embeds read the session they actually have.
///
/// The config-less [`default_embedding_provider`] hardcodes `(None, true)` and so
/// resolves `default_state_dir()` with encryption forced on; that only lands on
/// the right store for a default-root, encrypted, single-user install. Routing
/// the memory client's inline embedder through the keyless constructor is what
/// made a signed-in user's ingested documents persist vector-less — "Test
/// connection" passed (config-scoped) while the embed batch silently failed
/// (keyless scope) — #5501.
pub fn default_embedding_provider_with_config(config: &Config) -> Arc<dyn EmbeddingProvider> {
let (state_dir, encrypt_secrets) = managed_credential_scope(config);
// Never log `state_dir`: the user-scoped path embeds the OS username and/or
// `users/<uid>` (PII). Log only the non-identifying flag.
log::debug!(
"[embeddings::factory] building default managed embedder from config credential scope (encrypt={encrypt_secrets})"
);
Arc::new(OpenHumanCloudEmbedding::new(
None,
state_dir,
encrypt_secrets,
DEFAULT_CLOUD_EMBEDDING_MODEL,
DEFAULT_CLOUD_EMBEDDING_DIMENSIONS,
))
}

/// Returns the default embedding provider — cloud (OpenHuman backend, Voyage).
///
/// The cloud embedder lazily resolves the session JWT and API URL on each
/// call, so this can be constructed before login completes; the first
/// `embed()` will fail with a clear message if the user is unauthenticated.
///
/// **Keyless — prefer [`default_embedding_provider_with_config`].** This hardcodes
/// `(None, true)` for the credential scope, resolving `default_state_dir()`
/// (`~/.openhuman` root, or `users/<active>` post-#5427) with encryption forced
/// on. That reads the wrong store whenever the caller's config disables secret
/// encryption or roots the workspace/user elsewhere than the process default
/// (#5356 / #5501). Only callers that genuinely hold no `&Config` should use it.
pub fn default_embedding_provider() -> Arc<dyn EmbeddingProvider> {
Arc::new(OpenHumanCloudEmbedding::new(
None,
Expand Down Expand Up @@ -472,4 +511,92 @@ mod tests {
"managed provider must authenticate with the token from the config scope"
);
}

/// #5501 regression: the memory client's inline embedder is built through
/// [`default_embedding_provider_with_config`], which MUST authenticate with
/// the `app-session` token under the *config* credential scope — the same
/// scope "Test connection" uses. The pre-fix keyless
/// [`default_embedding_provider`] resolved `default_state_dir()` with
/// encryption forced on, so a signed-in user's ingested documents embedded
/// with "No backend session" and persisted vector-less while the connection
/// test still passed. A local mock stands in for the cloud backend (no
/// network) and captures the bearer it receives; a regression back to the
/// keyless constructor resolves a scope with no token and never reaches it.
#[tokio::test]
async fn default_provider_with_config_authenticates_with_config_scoped_token() {
use std::sync::{Arc, Mutex};

use axum::{extract::State, http::HeaderMap, routing::post, Json, Router};

#[derive(Clone, Default)]
struct Captured {
auth: Arc<Mutex<Option<String>>>,
}
let captured = Captured::default();
let app = Router::new()
.route(
"/openai/v1/embeddings",
post(
|State(cap): State<Captured>,
headers: HeaderMap,
Json(_body): Json<serde_json::Value>| async move {
*cap.auth.lock().unwrap() = headers
.get("authorization")
.and_then(|v| v.to_str().ok())
.map(str::to_owned);
// The embedder validates returned dims against the
// requested default, so the mock must echo that width.
let embedding = vec![0.1_f32; DEFAULT_CLOUD_EMBEDDING_DIMENSIONS];
Json(serde_json::json!({
"object": "list",
"data": [{ "object": "embedding", "index": 0, "embedding": embedding }],
"model": DEFAULT_CLOUD_EMBEDDING_MODEL,
}))
},
),
)
.with_state(captured.clone());
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let base = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port());
tokio::spawn(async move { axum::serve(listener, app).await.unwrap() });

// The app-session token lives ONLY under the config credential scope.
let tmp = TempDir::new().unwrap();
let config = test_config(&tmp);
AuthService::from_config(&config)
.store_provider_token(
APP_SESSION_PROVIDER,
"default",
"sess-default-5501",
HashMap::new(),
true,
)
.unwrap();

let provider = {
let _env_guard = crate::api::config::backend_env_test_lock();
let prev = std::env::var("BACKEND_URL").ok();
std::env::set_var("BACKEND_URL", &base);
let built = default_embedding_provider_with_config(&config);
match prev {
Some(v) => std::env::set_var("BACKEND_URL", v),
None => std::env::remove_var("BACKEND_URL"),
}
built
};

let vectors = provider
.embed(&["binding probe"])
.await
.expect("config-scoped default embedder must resolve the app-session token and embed");
assert_eq!(
vectors.first().map(|v| v.len()).unwrap_or(0),
DEFAULT_CLOUD_EMBEDDING_DIMENSIONS
);
assert_eq!(
captured.auth.lock().unwrap().as_deref(),
Some("Bearer sess-default-5501"),
"the memory client's default embedder must authenticate with the config-scoped token"
);
}
}
2 changes: 1 addition & 1 deletion src/openhuman/inference/embeddings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub use cloud::{
pub use factory::{
create_embedding_provider, create_embedding_provider_with_config,
create_embedding_provider_with_credentials, default_embedding_provider,
default_local_embedding_provider,
default_embedding_provider_with_config, default_local_embedding_provider,
};
// `pub(crate)` helper — reused by the memory-tree OpenAI-compat adapter to gate
// configs whose dimension the fixed-1024 tree can't store (#4056). Not part of
Expand Down
12 changes: 11 additions & 1 deletion src/openhuman/inference/embeddings/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1104,8 +1104,18 @@ mod tests {
config.memory.embedding_provider = "cloud".to_string();
// A managed session exists, so the ladder would resolve to cloud …
std::fs::write(tmp.path().join("auth-profiles.json"), "{}").unwrap();
// … except the unified workload setting routes embeddings to Ollama.
// … except a local Ollama route wins. As of tinymemory v1.0.1 the
// effective-embedder ladder no longer treats the `embeddings_provider`
// string alone as authoritative for local routing — local Ollama is
// resolved from an explicit `memory_tree.embedding_endpoint` override or
// the unified `workload_local_model` setting. Drive the explicit
// endpoint rung here: it resolves deterministically without an installed
// embedding host, and still exercises the point of the test — that
// `provider` (the picker) stays `cloud` while `effective_provider`
// reports the local route that bills nothing (#5402).
config.embeddings_provider = Some("ollama:all-minilm:latest".into());
config.memory_tree.embedding_endpoint = Some("http://localhost:11434".into());
config.memory_tree.embedding_model = Some("all-minilm".into());

let out = get_settings(&config)
.await
Expand Down
10 changes: 9 additions & 1 deletion src/openhuman/memory/host_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,15 @@ impl EmbeddingHost for OpenHumanEmbeddingHost {
}

fn default_embedding_provider(&self) -> Arc<dyn EmbeddingProvider> {
crate::openhuman::inference::embeddings::default_embedding_provider()
// Scope the managed embedder to THIS host's config credential store, not
// the keyless `default_state_dir()` hardcode. The memory client caches
// this provider for the process lifetime, so a keyless scope that misses
// the signed-in user's `app-session` token makes every ingested
// document persist vector-less while "Test connection" (config-scoped)
// still passes — #5501.
crate::openhuman::inference::embeddings::default_embedding_provider_with_config(
&self.config,
)
}

fn create_embedding_provider_with_credentials(
Expand Down
Loading