fix(memory): thread BYO embedding credential — stop Cohere empty-key 401 flood (#3354) (#3358)

src/core/observability.rs

@@ -263,9 +263,17 @@ pub fn expected_error_kind(message: &str) -> Option<ExpectedErrorKind> {
     // no longer reaches here — but other direct-mode ops the user invokes
     // explicitly (execute / authorize) can still surface it, and it is the
     // same user-config state with no Sentry-actionable signal.
+    //
+    // `"no api key supplied"` is Cohere's exact 401 body wording when the
+    // hosted `/v2/embed` endpoint is called with an empty bearer (the BYO-key
+    // embedding path with no key configured). The `CohereEmbedding::embed`
+    // guard now fast-fails before issuing that request, but this arm demotes
+    // any residual 401 of that shape — older clients, or a present-but-rejected
+    // key — so the flood (TAURI-RUST-52S) stays out of Sentry either way.
     if lower.contains("api key not set")
         || lower.contains("missing api key")
         || lower.contains("no api key is configured")
+        || lower.contains("no api key supplied")
     {
         return Some(ExpectedErrorKind::ApiKeyMissing);
     }
@@ -2048,6 +2056,23 @@ mod tests {
         );
     }
 
+    /// Sentry TAURI-RUST-52S: Cohere's hosted `/v2/embed` returns a 401 with
+    /// body `"no api key supplied"` when called with an empty bearer (BYO-key
+    /// embedding path, no key configured). Verbatim wire shape from the embed
+    /// client must classify as `ApiKeyMissing` so the 8.7k-event flood stays
+    /// out of Sentry even from clients that predate the call-site guard.
+    #[test]
+    fn classifies_cohere_missing_api_key_401_as_api_key_missing() {
+        assert_eq!(
+            expected_error_kind(
+                "Cohere embed API error (401 Unauthorized): \
+                 {\"id\":\"3176e92f-d18e-4019-bd43-314821504a61\",\
+                 \"message\":\"no api key supplied\"}"
+            ),
+            Some(ExpectedErrorKind::ApiKeyMissing)
+        );
+    }
+
     /// Guard against over-suppression: a genuine BYO-key auth failure
     /// (wrong/expired key the upstream rejected) is an actionable bug
     /// shape and MUST still reach Sentry (stay `None`), not get demoted by

src/openhuman/agent/harness/session/builder.rs

@@ -864,9 +864,14 @@ impl Agent {
         )?;
 
         let local_embedding = config.workload_local_model("embeddings");
+        let embedding_api_key = crate::openhuman::embeddings::resolve_api_key(
+            config,
+            &config.memory.embedding_provider,
+        );
         let memory: Arc<dyn Memory> = Arc::from(memory_store::create_memory_with_local_ai(
             &config.memory,
             local_embedding.as_deref(),
+            &embedding_api_key,
             &config.embedding_routes,
             Some(&config.storage.provider.config),
             &config.workspace_dir,

src/openhuman/channels/runtime/startup.rs

@@ -329,9 +329,12 @@ pub async fn start_channels(mut config: Config) -> Result<()> {
     )?;
     let temperature = config.default_temperature;
     let local_embedding = config.workload_local_model("embeddings");
+    let embedding_api_key =
+        crate::openhuman::embeddings::resolve_api_key(&config, &config.memory.embedding_provider);
     let mem: Arc<dyn Memory> = Arc::from(memory_store::create_memory_with_local_ai(
         &config.memory,
         local_embedding.as_deref(),
+        &embedding_api_key,
         &[],
         Some(&config.storage.provider.config),
         &config.workspace_dir,

src/openhuman/embeddings/cohere.rs

@@ -95,6 +95,25 @@ impl EmbeddingProvider for CohereEmbedding {
             return Ok(Vec::new());
         }
 
+        // Fast-fail when no API key is configured. Cohere's hosted `/v2/embed`
+        // always requires a bearer token; without this guard we POST
+        // `Authorization: Bearer ` (empty) and Cohere returns a 401 "no api key
+        // supplied" on every call. That 401 is not retryable, so each embed
+        // attempt bails and reports an error — the memory pipeline re-embeds per
+        // document and floods Sentry (TAURI-RUST-52S: 8.7k events from a single
+        // misconfigured user). Bailing here skips the wasted request entirely,
+        // and the "API key not set" wording is demoted to a breadcrumb by the
+        // `ApiKeyMissing` classifier in
+        // `core::observability::expected_error_kind` instead of surfacing as a
+        // Sentry event. Scoped to Cohere on purpose: the OpenAI-compatible
+        // provider legitimately supports keyless local/custom endpoints, so it
+        // omits the header rather than bailing.
+        if self.api_key.trim().is_empty() {
+            anyhow::bail!(
+                "Cohere API key not set. Configure via the web UI or set the appropriate env var."
+            );
+        }
+
         let url = format!("{}/v2/embed", self.base_url);
 
         tracing::debug!(
@@ -253,6 +272,24 @@ mod tests {
         assert!(p.embed(&[]).await.unwrap().is_empty());
     }
 
+    /// Missing / whitespace-only API key bails before any HTTP request with the
+    /// classifiable "API key not set" wording (TAURI-RUST-52S). `with_base_url`
+    /// points at an address nothing is listening on, so the assertion that the
+    /// error is the key-guard message — not a connection error — proves no
+    /// request was attempted.
+    #[tokio::test]
+    async fn embed_missing_api_key_bails_without_request() {
+        for key in ["", "   "] {
+            let p = CohereEmbedding::new(key, "embed-english-v3.0", 1024)
+                .with_base_url("http://127.0.0.1:1");
+            let err = p.embed(&["hello"]).await.unwrap_err().to_string();
+            assert!(
+                err.contains("API key not set"),
+                "expected key-guard message for key {key:?}, got: {err}"
+            );
+        }
+    }
+
     // ── 429 backoff tests ──────────────────────────────────────
 
     use axum::{http::StatusCode, routing::post, Router};

src/openhuman/embeddings/rpc.rs

@@ -414,3 +414,67 @@ pub(crate) fn resolve_api_key(config: &Config, provider_name: &str) -> String {
         .flatten()
         .unwrap_or_default()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use tempfile::TempDir;
+
+    /// The seam the memory factory depends on (TAURI-RUST-52S fix): the three
+    /// `create_memory_with_local_ai` call sites resolve the user's stored BYO
+    /// embedding credential via `resolve_api_key` and thread it into the
+    /// provider. If this lookup silently returns "" for a configured key —
+    /// wrong cred slug, encryption mismatch, profile-store regression — the
+    /// memory pipeline reverts to sending an empty bearer and Cohere 401s on
+    /// every embed. Lock the round-trip: store under `embeddings:<slug>`, read
+    /// it back; an unrelated provider must stay empty (no cross-bleed).
+    #[test]
+    fn resolve_api_key_returns_stored_embeddings_credential() {
+        let tmp = TempDir::new().unwrap();
+        let mut config = Config::default();
+        config.config_path = tmp.path().join("config.toml");
+
+        // Nothing stored yet → empty (the empty-key guard's "" input).
+        assert_eq!(resolve_api_key(&config, "cohere"), "");
+
+        // Store a Cohere embeddings key exactly as `set_api_key` does.
+        AuthService::from_config(&config)
+            .store_provider_token(
+                "embeddings:cohere",
+                "default",
+                "sk-cohere-test",
+                HashMap::new(),
+                true,
+            )
+            .unwrap();
+
+        // Resolve returns it; a provider with no stored key stays empty.
+        assert_eq!(resolve_api_key(&config, "cohere"), "sk-cohere-test");
+        assert_eq!(resolve_api_key(&config, "voyage"), "");
+    }
+
+    /// `custom:<url>` providers must look up under the `embeddings:custom`
+    /// slug (the inline URL is not part of the credential key), mirroring the
+    /// slug normalization in `embed`/`set_api_key`.
+    #[test]
+    fn resolve_api_key_normalizes_custom_prefix_to_custom_slug() {
+        let tmp = TempDir::new().unwrap();
+        let mut config = Config::default();
+        config.config_path = tmp.path().join("config.toml");
+
+        AuthService::from_config(&config)
+            .store_provider_token(
+                "embeddings:custom",
+                "default",
+                "sk-custom-test",
+                HashMap::new(),
+                true,
+            )
+            .unwrap();
+
+        assert_eq!(
+            resolve_api_key(&config, "custom:http://localhost:1234"),
+            "sk-custom-test"
+        );
+    }
+}

src/openhuman/memory_store/factories.rs

@@ -298,7 +298,10 @@ pub fn create_memory(
     config: &MemoryConfig,
     workspace_dir: &Path,
 ) -> anyhow::Result<Box<dyn Memory>> {
-    create_memory_full(config, &[], None, None, workspace_dir)
+    // No `Config` in scope here (tests + migration), so no credential store to
+    // read — pass an empty key. Callers that select a keyed BYO provider must
+    // use `create_memory_with_local_ai`, which resolves the stored credential.
+    create_memory_full(config, &[], None, None, "", workspace_dir)
 }
 
 /// Create a memory instance honouring the unified per-workload embedding
@@ -309,9 +312,17 @@ pub fn create_memory(
 /// `None`. Used by top-level entry points (agent harness, channels runtime)
 /// that have the full `Config` in scope. The local-AI opt-in flips the
 /// embedder to Ollama when `Some`.
+///
+/// `embedding_api_key` is the user's stored credential for the selected BYO
+/// embedding provider, resolved by the caller via
+/// [`crate::openhuman::embeddings::resolve_api_key`] (empty string when none is
+/// configured). It is threaded into the keyed providers (cohere/openai/voyage/
+/// custom) so they authenticate instead of sending an empty bearer; cloud /
+/// managed / ollama / none ignore it.
 pub fn create_memory_with_local_ai(
     memory: &MemoryConfig,
     local_embedding_model: Option<&str>,
+    embedding_api_key: &str,
     embedding_routes: &[EmbeddingRouteConfig],
     storage_provider: Option<&StorageProviderConfig>,
     workspace_dir: &Path,
@@ -321,6 +332,7 @@ pub fn create_memory_with_local_ai(
         embedding_routes,
         storage_provider,
         local_embedding_model,
+        embedding_api_key,
         workspace_dir,
     )
 }
@@ -370,6 +382,7 @@ fn create_memory_full(
     _embedding_routes: &[EmbeddingRouteConfig],
     _storage_provider: Option<&StorageProviderConfig>,
     local_embedding_model: Option<&str>,
+    embedding_api_key: &str,
     workspace_dir: &Path,
 ) -> anyhow::Result<Box<dyn Memory>> {
     // 1. Resolve the intended provider from config.
@@ -413,11 +426,29 @@ fn create_memory_full(
          (local_ai_opt_in={local_ai_opt_in} gate_triggered={gate_triggered})",
     );
 
-    // 3. Create the embedding provider.
+    // 3. Create the embedding provider, threading the user's stored BYO
+    //    credential. The keyless `create_embedding_provider` left the API key
+    //    empty for *every* provider, so a user who selected Cohere — even with
+    //    a valid key configured — sent an empty `Bearer ` and got a guaranteed
+    //    401 "no api key supplied" on every embed (TAURI-RUST-52S), and the
+    //    same gap silently broke BYO OpenAI / Voyage memory embeddings.
+    //    cloud/managed/ollama/none ignore the key; the keyed providers now
+    //    actually receive it. `embedding_api_key` is "" when no credential is
+    //    stored, which the per-provider guards reject fast. A `custom:<url>`
+    //    provider keeps its inline endpoint (the factory's `custom:` arm strips
+    //    the prefix), so `custom_endpoint` stays `None` here. The key is never
+    //    logged — the warning carries only provider/model/dims.
     let embedder: Arc<dyn EmbeddingProvider> = Arc::from(
-        embeddings::create_embedding_provider(&provider, &model, dims).inspect_err(|err| {
+        embeddings::create_embedding_provider_with_credentials(
+            &provider,
+            &model,
+            dims,
+            embedding_api_key,
+            None,
+        )
+        .inspect_err(|err| {
             log::warn!(
-                "[memory::factory] create_embedding_provider failed provider={provider} model={model} dims={dims}: {err}",
+                "[memory::factory] create_embedding_provider_with_credentials failed provider={provider} model={model} dims={dims}: {err}",
             );
         })?,
     );

src/openhuman/runtime_node/ops.rs

@@ -48,11 +48,14 @@ fn build_runtime_tools(config: &Config) -> Result<Vec<Box<dyn Tool>>, String> {
     .map_err(|e| e.to_string())?;
     let runtime: Arc<dyn RuntimeAdapter> = Arc::new(NativeRuntime::new());
     let local_embedding = config.workload_local_model("embeddings");
+    let embedding_api_key =
+        crate::openhuman::embeddings::resolve_api_key(config, &config.memory.embedding_provider);
     trace!("[runtime_node::ops] build_runtime_tools: create_memory_with_local_ai");
     let memory: Arc<dyn Memory> = Arc::from(
         crate::openhuman::memory_store::create_memory_with_local_ai(
             &config.memory,
             local_embedding.as_deref(),
+            &embedding_api_key,
             &config.embedding_routes,
             Some(&config.storage.provider.config),
             &config.workspace_dir,