From 27739ee59e649653cd5f2430ab44de22a726baa1 Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Mon, 15 Jun 2026 10:36:06 +0530 Subject: [PATCH] fix(embeddings): prevent + handle custom endpoint with no embeddings API (Sentry TAURI-RUST-5JR) (#3625) (#3629) Co-authored-by: Steven Enamakel --- .../settings/panels/EmbeddingsPanel.tsx | 12 +++ .../panels/__tests__/EmbeddingsPanel.test.tsx | 32 ++++++ src/core/observability.rs | 79 +++++++++++++++ src/openhuman/embeddings/openai.rs | 16 ++- src/openhuman/embeddings/openai_tests.rs | 30 ++++++ src/openhuman/embeddings/rpc.rs | 88 ++++++++++++++++- tests/embeddings_rpc_e2e.rs | 99 ++++++++++++++++++- 7 files changed, 348 insertions(+), 8 deletions(-) diff --git a/app/src/components/settings/panels/EmbeddingsPanel.tsx b/app/src/components/settings/panels/EmbeddingsPanel.tsx index 27dabc1de..f17434c86 100644 --- a/app/src/components/settings/panels/EmbeddingsPanel.tsx +++ b/app/src/components/settings/panels/EmbeddingsPanel.tsx @@ -282,6 +282,18 @@ const EmbeddingsPanel = ({ embedded = false }: EmbeddingsPanelProps = {}) => { custom_endpoint: customEndpoint.trim(), confirm_wipe: false, }); + // The endpoint responded but exposes no embeddings API (e.g. a chat-only + // base URL like DeepSeek). Keep the setup popup open and surface the + // actionable message so the user can correct the endpoint — TAURI-RUST-5JR. + if (result.error === 'EMBEDDINGS_ENDPOINT_NO_API') { + setSetupError( + typeof result.message === 'string' + ? result.message + : 'This endpoint has no embeddings API. Choose an embeddings-capable provider or a different endpoint.' + ); + setStatus({ kind: 'idle' }); + return; + } if (result.error === 'EMBEDDINGS_DIMENSION_CHANGE_REQUIRES_WIPE') { setPendingWipe({ provider: 'custom', diff --git a/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx b/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx index 5a584d947..b8f8ae4e6 100644 --- a/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx +++ b/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx @@ -352,6 +352,38 @@ describe('EmbeddingsPanel', () => { ); }); + it('surfaces an actionable error and keeps the popup open when the custom endpoint has no embeddings API', async () => { + // TAURI-RUST-5JR: the backend probes the endpoint and rejects a chat-only + // URL (DeepSeek) with EMBEDDINGS_ENDPOINT_NO_API. The panel must show the + // message and NOT close the setup popup, so the user can fix the endpoint. + const settings = makeSettings({ + providers: [ + makeProvider('managed', { requires_api_key: false }), + makeProvider('custom', { requires_api_key: false, requires_endpoint: true }), + ], + }); + vi.mocked(loadEmbeddingsSettings).mockResolvedValue(settings); + vi.mocked(updateEmbeddingsSettings).mockResolvedValue({ + error: 'EMBEDDINGS_ENDPOINT_NO_API', + message: 'This endpoint has no embeddings API. Choose an embeddings-capable provider.', + }); + + renderWithProviders(); + await screen.findByText('Custom'); + + fireEvent.click(screen.getByRole('radio', { name: /custom/i })); + await screen.findByPlaceholderText(/https:\/\/your-endpoint/i); + fireEvent.change(screen.getByPlaceholderText(/https:\/\/your-endpoint/i), { + target: { value: 'https://api.deepseek.com/v1' }, + }); + fireEvent.click(screen.getByRole('button', { name: /save.*switch/i })); + + // Actionable message shown. + await screen.findByText(/no embeddings API/i); + // Popup stays open — endpoint input is still present so the user can fix it. + expect(screen.getByPlaceholderText(/https:\/\/your-endpoint/i)).toBeInTheDocument(); + }); + // ─── Confirm wipe dialog ────────────────────────────────────────────────── it('shows confirm-wipe dialog when updateEmbeddingsSettings returns DIMENSION_CHANGE error', async () => { diff --git a/src/core/observability.rs b/src/core/observability.rs index 854f9e9a6..6537459cf 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -417,6 +417,16 @@ pub fn expected_error_kind(message: &str) -> Option { if is_embedding_backend_auth_failure(&lower) { return Some(ExpectedErrorKind::SessionExpired); } + // TAURI-RUST-5JR — a custom embeddings endpoint with no embeddings route + // (the user pointed the Custom (OpenAI-compatible) provider at a chat-only + // base URL, e.g. DeepSeek, which 404s every `/embeddings` POST). + // Deterministic user-config state, re-emitted on every memory re-embed; + // the embeddings settings UI surfaces an actionable "pick an + // embeddings-capable provider" message. Demote to info. Scoped to 404/405 + // only so a real 500 from a valid embeddings endpoint stays in Sentry. + if is_embedding_endpoint_absent(&lower) { + return Some(ExpectedErrorKind::ProviderConfigRejection); + } // Provider config-rejection (unknown model / abstract tier leaked to a // custom provider / model-specific temperature). Body-shape based and // intrinsically scoped to third-party providers — the OpenHuman @@ -557,6 +567,32 @@ fn is_embedding_backend_auth_failure(lower: &str) -> bool { && lower.contains("invalid token") } +/// Detect a custom embeddings endpoint that exposes **no embeddings API** — +/// the `OpenAiEmbedding` client POSTed `/embeddings` and the host answered +/// `404 Not Found` (route absent) or `405 Method Not Allowed`. Canonical wire +/// shape from `src/openhuman/embeddings/openai.rs`: +/// +/// ```text +/// Embedding API error (404 Not Found): +/// Embedding API error (405 Method Not Allowed): +/// ``` +/// +/// Deterministic user-config state: the user pointed the Custom +/// (OpenAI-compatible) embeddings provider at a base URL whose host has no +/// embeddings endpoint (e.g. a chat-only provider like DeepSeek). Every memory +/// re-embed re-emits it (TAURI-RUST-5JR, ~2685 events / 9 users) and the +/// settings UI surfaces an actionable remediation — Sentry has no fix to make. +/// +/// Polarity (important): scoped to **404/405 only**. A `500` from a valid +/// embeddings endpoint is a real server fault and must keep reaching Sentry; a +/// `400` (e.g. oversized input) is prevented at source by the chunk cap +/// (#3598) and likewise stays visible. Reused by +/// `embeddings::rpc::update_settings` as the save-time hard-block signal so the +/// two never drift. +pub(crate) fn is_embedding_endpoint_absent(lower: &str) -> bool { + lower.contains("embedding api error") && (lower.contains("(404") || lower.contains("(405")) +} + /// Detect the memory-store chunk DB's circuit-breaker-open message that /// `memory_store::chunks::store::get_or_init_connection` emits via /// `anyhow::bail!` when the per-path breaker rejects new init attempts. @@ -3771,6 +3807,49 @@ mod tests { ); } + #[test] + fn classifies_embedding_endpoint_absent_as_config_rejection() { + // TAURI-RUST-5JR — custom embeddings provider pointed at a chat-only + // base URL (DeepSeek) that has no `/embeddings` route. Verbatim shape + // produced by `src/openhuman/embeddings/openai.rs` (prefix preserved + // even after the actionable-hint suffix is appended). + assert_eq!( + expected_error_kind( + "Embedding API error (404 Not Found): not found \ + — this endpoint has no embeddings API; pick an embeddings-capable \ + provider in Settings → Memory" + ), + Some(ExpectedErrorKind::ProviderConfigRejection) + ); + // 405 Method Not Allowed — route exists for GET only / wrong verb. + assert_eq!( + expected_error_kind("Embedding API error (405 Method Not Allowed): {}"), + Some(ExpectedErrorKind::ProviderConfigRejection) + ); + } + + #[test] + fn does_not_demote_real_embedding_server_faults() { + // Polarity guard: a 500 from a VALID embeddings endpoint is a real + // server fault and must keep reaching Sentry — not demoted. + assert_eq!( + expected_error_kind("Embedding API error (500 Internal Server Error): upstream boom"), + None, + "embedding 500 is a real fault and must stay in Sentry" + ); + // A 400 (e.g. oversized input — TAURI-RUST-4SA) is prevented at source + // by the chunk cap (#3598); a residual 400 must stay visible, NOT be + // swallowed by the 404/405-scoped endpoint-absent arm. + assert_eq!( + expected_error_kind( + "Embedding API error (400 Bad Request): {\"error\":{\"message\":\ + \"maximum input length is 8192 tokens.\"}}" + ), + None, + "embedding 400 must NOT be demoted by the endpoint-absent (404/405) arm" + ); + } + #[test] fn does_not_classify_unrelated_provider_failures_as_config_rejection() { // Inverted polarity / scope guard: a 5xx or a generic 4xx with no diff --git a/src/openhuman/embeddings/openai.rs b/src/openhuman/embeddings/openai.rs index a3a289432..a645c48bf 100644 --- a/src/openhuman/embeddings/openai.rs +++ b/src/openhuman/embeddings/openai.rs @@ -207,7 +207,21 @@ impl EmbeddingProvider for OpenAiEmbedding { target: "openai::embed", "[openai] embed error: status={status}, body={text}" ); - let message = format!("Embedding API error ({status}): {text}"); + let mut message = format!("Embedding API error ({status}): {text}"); + // A 404/405 means the base URL responded but exposes no + // embeddings route — the user pointed the Custom + // (OpenAI-compatible) provider at a chat-only endpoint (e.g. + // DeepSeek). Append an actionable remediation while PRESERVING + // the `Embedding API error (404…)` prefix that + // `observability::is_embedding_endpoint_absent` keys on, so the + // event is still demoted from Sentry. Host-agnostic text (no + // URL/credential echo). TAURI-RUST-5JR. + if matches!(status.as_u16(), 404 | 405) { + message.push_str( + " — this endpoint has no embeddings API; pick an \ + embeddings-capable provider in Settings → Memory", + ); + } // Use `report_error_or_expected` so transient upstream HTTP // failures (e.g. 429 Too Many Requests after retry cap) log a // warning breadcrumb instead of firing a Sentry error event. diff --git a/src/openhuman/embeddings/openai_tests.rs b/src/openhuman/embeddings/openai_tests.rs index 6b1074d4b..f7b6490ee 100644 --- a/src/openhuman/embeddings/openai_tests.rs +++ b/src/openhuman/embeddings/openai_tests.rs @@ -263,6 +263,36 @@ async fn embed_server_error() { assert!(msg.contains("rate limited"), "body: {msg}"); } +/// A 404 means the configured base URL has no embeddings route (the user +/// pointed the Custom provider at a chat-only endpoint, e.g. DeepSeek — +/// TAURI-RUST-5JR). The message must (a) carry an actionable remediation, and +/// (b) PRESERVE the `Embedding API error (404…)` prefix the +/// `observability::is_embedding_endpoint_absent` classifier keys on, so the +/// flood is demoted from Sentry rather than firing on every re-embed. +#[tokio::test] +async fn embed_404_endpoint_absent_is_actionable_and_classifier_stable() { + let app = Router::new().route( + "/v1/embeddings", + post(|| async { (StatusCode::NOT_FOUND, "Not Found") }), + ); + let url = start_mock(app).await; + let p = OpenAiEmbedding::new(&url, "k", "m", 1); + + let err = p.embed(&["hi"]).await.unwrap_err(); + let msg = err.to_string(); + // Classifier contract: prefix preserved. + assert!( + msg.to_ascii_lowercase() + .contains("embedding api error (404"), + "must preserve the (404 classifier prefix: {msg}" + ); + // Actionable remediation appended. + assert!( + msg.contains("no embeddings API") && msg.contains("Settings → Memory"), + "must carry actionable remediation: {msg}" + ); +} + /// 429 rate-limit responses must format their message in the canonical /// `"... API error (): "` shape so the shared /// `is_transient_upstream_http_message` classifier in `core::observability` diff --git a/src/openhuman/embeddings/rpc.rs b/src/openhuman/embeddings/rpc.rs index 52bce4d4c..b33ca9a29 100644 --- a/src/openhuman/embeddings/rpc.rs +++ b/src/openhuman/embeddings/rpc.rs @@ -109,6 +109,70 @@ pub async fn update_settings( let dims_changed = new_dims != old_dims; let sig_changed = new_sig != old_sig; + // Prevention (TAURI-RUST-5JR): a Custom (OpenAI-compatible) endpoint whose + // host has no `/embeddings` route (e.g. a chat-only provider like DeepSeek) + // 404s every memory re-embed forever — 2685 Sentry events / 9 users before + // this gate. Probe the endpoint ONCE here so a no-embeddings URL can never + // be persisted (and we never wipe memory for a config that can't embed). + // Only custom endpoints are probed: named catalog providers are + // embedding-capable by construction, and probing `managed`/`cloud` + // pre-login would false-fail. Resolve the provider string exactly as it + // will be stored so the probe targets the real endpoint. + let effective_provider = match &custom_endpoint { + Some(ep) if new_provider == "custom" || new_provider.starts_with("custom:") => { + format!("custom:{ep}") + } + _ => new_provider.clone(), + }; + if effective_provider.starts_with("custom:") { + match build_embedder(&config, &effective_provider, &new_model, new_dims) { + Ok(embedder) => { + // Time-box the probe so a black-hole host can't hang the RPC. + let probe = tokio::time::timeout( + std::time::Duration::from_secs(10), + embedder.embed(&["connection test"]), + ) + .await; + if let Ok(Err(e)) = probe { + let detail = e.to_string(); + // HARD-block only the deterministic endpoint-absent shape + // (404/405). Transient failures (timeout/5xx/network) fall + // through and save — never lock out a valid-but-down + // endpoint; the embed-time classifier handles any residual. + if crate::core::observability::is_embedding_endpoint_absent( + &detail.to_ascii_lowercase(), + ) { + tracing::warn!( + provider = effective_provider.as_str(), + "{LOG_PREFIX} update_settings rejected — endpoint has no embeddings API" + ); + let payload = serde_json::json!({ + "error": "EMBEDDINGS_ENDPOINT_NO_API", + "message": "This endpoint has no embeddings API. Choose an \ + embeddings-capable provider (Managed, Voyage, OpenAI, \ + Cohere, Ollama) or a different custom endpoint.", + "detail": detail, + }); + return Ok(RpcOutcome::new( + payload, + vec!["embeddings endpoint has no embeddings API — not saved".into()], + )); + } + tracing::warn!( + provider = effective_provider.as_str(), + error = detail.as_str(), + "{LOG_PREFIX} update_settings probe inconclusive — saving anyway" + ); + } + } + Err(e) => { + // Construction failure (unknown slug / bad config) — surface it + // rather than persisting a config that can never embed. + return Err(format!("invalid embedding provider configuration: {e}")); + } + } + } + // Only require a wipe when dimensions actually change — switching // provider/model at the same dimensionality keeps vectors comparable. if dims_changed && !confirm_wipe { @@ -382,15 +446,31 @@ pub async fn test_connection( /// [`embed`] uses, exposed so other domains (e.g. `codegraph`) can obtain a /// provider for `signature()` + direct embedding without a JSON-RPC round-trip. pub fn provider_from_config(config: &Config) -> anyhow::Result> { - let provider_name = &config.memory.embedding_provider; - let model = &config.memory.embedding_model; - let dims = config.memory.embedding_dimensions; + build_embedder( + config, + &config.memory.embedding_provider, + &config.memory.embedding_model, + config.memory.embedding_dimensions, + ) +} + +/// Construct an embedding provider for an explicit `(provider_name, model, +/// dims)` triple, resolving the stored API key + inline `custom:` endpoint +/// the same way [`embed`] / [`test_connection`] do. Single construction seam so +/// the save-time probe in [`update_settings`] and the live embed path can't +/// drift on slug-normalization / credential-lookup rules. +fn build_embedder( + config: &Config, + provider_name: &str, + model: &str, + dims: usize, +) -> anyhow::Result> { let api_key = resolve_api_key(config, provider_name); let custom_endpoint = provider_name.strip_prefix("custom:").map(|s| s.to_string()); let provider_slug = if provider_name.starts_with("custom:") { "custom" } else { - provider_name.as_str() + provider_name }; create_embedding_provider_with_credentials( provider_slug, diff --git a/tests/embeddings_rpc_e2e.rs b/tests/embeddings_rpc_e2e.rs index b9ace432e..3a840b743 100644 --- a/tests/embeddings_rpc_e2e.rs +++ b/tests/embeddings_rpc_e2e.rs @@ -645,13 +645,24 @@ async fn embeddings_embed_with_custom_openai_endpoint_round_trips_vectors_and_ap ); let requests = mock_state.requests.lock().expect("mock requests lock"); - assert_eq!(requests.len(), 1, "custom endpoint should be called once"); + // Two hits: (0) the save-time connectivity probe update_settings now runs + // against custom endpoints (TAURI-RUST-5JR prevention), (1) the real embed. assert_eq!( - requests[0].get("model").and_then(Value::as_str), - Some("mock-embedding-model") + requests.len(), + 2, + "expected one save-time probe + one embed call" ); assert_eq!( requests[0].pointer("/input/0").and_then(Value::as_str), + Some("connection test"), + "first call must be the save-time validation probe" + ); + assert_eq!( + requests[1].get("model").and_then(Value::as_str), + Some("mock-embedding-model") + ); + assert_eq!( + requests[1].pointer("/input/0").and_then(Value::as_str), Some("first custom text") ); drop(requests); @@ -668,6 +679,88 @@ async fn embeddings_embed_with_custom_openai_endpoint_round_trips_vectors_and_ap mock_join.abort(); } +/// A mock "OpenAI-compatible" host that has NO embeddings route — every POST to +/// `/v1/embeddings` 404s, exactly like a chat-only provider (DeepSeek) does. +async fn serve_mock_embeddings_no_api( +) -> (String, tokio::task::JoinHandle>) { + async fn not_found() -> (axum::http::StatusCode, &'static str) { + (axum::http::StatusCode::NOT_FOUND, "Not Found") + } + let router = Router::new().route("/v1/embeddings", post(not_found)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind no-api mock server"); + let addr = listener.local_addr().expect("no-api mock local_addr"); + let join = tokio::spawn(async move { axum::serve(listener, router).await }); + (format!("http://{addr}"), join) +} + +/// TAURI-RUST-5JR prevention: `update_settings` must probe a custom endpoint +/// and REFUSE to persist one that has no embeddings API (404), returning +/// `EMBEDDINGS_ENDPOINT_NO_API` and leaving the stored provider unchanged — so +/// the 404-on-every-re-embed Sentry flood can never be configured. +#[tokio::test(flavor = "multi_thread")] +async fn embeddings_update_settings_rejects_endpoint_with_no_embeddings_api() { + let _lock = embeddings_e2e_env_lock(); + let (rpc_base, _tmp, _guards, _join) = setup_embeddings_test().await; + let (mock_base, mock_join) = serve_mock_embeddings_no_api().await; + + // Snapshot the provider before the rejected save. + let before = post_json_rpc( + &rpc_base, + 80, + "openhuman.embeddings_get_settings", + json!({}), + ) + .await; + let before_result = assert_no_rpc_error(&before, "get_settings before"); + let before_inner = before_result.get("result").unwrap_or(before_result); + let before_provider = before_inner + .get("provider") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(); + + let update = post_json_rpc( + &rpc_base, + 81, + "openhuman.embeddings_update_settings", + json!({ + "provider": "custom", + "custom_endpoint": mock_base, + "model": "mock-embedding-model", + "dimensions": 3, + "confirm_wipe": true + }), + ) + .await; + let update_result = assert_no_rpc_error(&update, "update_settings no-api endpoint"); + let update_inner = update_result.get("result").unwrap_or(update_result); + assert_eq!( + update_inner.get("error").and_then(Value::as_str), + Some("EMBEDDINGS_ENDPOINT_NO_API"), + "a no-embeddings endpoint must be rejected, not saved: {update_inner}" + ); + + // The stored provider must be unchanged — nothing was persisted. + let after = post_json_rpc( + &rpc_base, + 82, + "openhuman.embeddings_get_settings", + json!({}), + ) + .await; + let after_result = assert_no_rpc_error(&after, "get_settings after"); + let after_inner = after_result.get("result").unwrap_or(after_result); + assert_eq!( + after_inner.get("provider").and_then(Value::as_str), + Some(before_provider.as_str()), + "provider must be unchanged after a rejected save: {after_inner}" + ); + + mock_join.abort(); +} + #[tokio::test(flavor = "multi_thread")] async fn legacy_alias_inference_embed_resolves() { let _lock = embeddings_e2e_env_lock();