diff --git a/app/src/components/settings/panels/EmbeddingsPanel.tsx b/app/src/components/settings/panels/EmbeddingsPanel.tsx index f17434c86..97bef7703 100644 --- a/app/src/components/settings/panels/EmbeddingsPanel.tsx +++ b/app/src/components/settings/panels/EmbeddingsPanel.tsx @@ -282,14 +282,21 @@ 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') { + // Setup-time verification failed: the endpoint couldn't prove it can + // embed, so the config was NOT saved. Covers no `/embeddings` route + // (TAURI-RUST-5JR), LM Studio with no model loaded (TAURI-RUST-4P4), and + // any other probe failure/timeout. Keep the setup popup open and surface + // the actionable message so the user can fix it (load a model, correct the + // endpoint, …) and retry. + if ( + result.error === 'EMBEDDINGS_ENDPOINT_NO_API' || + result.error === 'EMBEDDINGS_NO_MODEL_LOADED' || + result.error === 'EMBEDDINGS_VERIFICATION_FAILED' + ) { setSetupError( typeof result.message === 'string' ? result.message - : 'This endpoint has no embeddings API. Choose an embeddings-capable provider or a different endpoint.' + : "Couldn't verify the embeddings endpoint. Make sure it's running and serving an embedding model, then save again." ); setStatus({ kind: 'idle' }); return; diff --git a/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx b/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx index b8f8ae4e6..705352266 100644 --- a/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx +++ b/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx @@ -384,6 +384,71 @@ describe('EmbeddingsPanel', () => { expect(screen.getByPlaceholderText(/https:\/\/your-endpoint/i)).toBeInTheDocument(); }); + it('surfaces the no-model-loaded message and keeps the popup open when LM Studio has no model loaded', async () => { + // TAURI-RUST-4P4: the backend runs a setup-time test embed and rejects an + // LM Studio endpoint with no model loaded (EMBEDDINGS_NO_MODEL_LOADED). The + // panel must show the one-step remediation and NOT close the popup, so the + // user can load a model and retry — verifying at setup is the fix. + 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_NO_MODEL_LOADED', + message: + 'Your local embeddings server (e.g. LM Studio) is running but has no model loaded. Load an embedding model, then save again.', + }); + + 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: 'http://localhost:1234/v1' }, + }); + fireEvent.click(screen.getByRole('button', { name: /save.*switch/i })); + + // Actionable remediation shown. + await screen.findByText(/no model loaded/i); + // Popup stays open so the user can fix it and retry. + expect(screen.getByPlaceholderText(/https:\/\/your-endpoint/i)).toBeInTheDocument(); + }); + + it('surfaces a verification-failed message and keeps the popup open when the test embed fails', async () => { + // The setup-time test embed failed (timeout / 5xx / unreachable). The + // config is NOT saved; the panel surfaces the generic verification message + // and keeps the popup open so the user can fix the endpoint and retry. + 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_VERIFICATION_FAILED', + message: + "Couldn't verify the embeddings endpoint — the test embed failed. Make sure the endpoint is reachable, then save again.", + }); + + 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: 'http://localhost:9/v1' }, + }); + fireEvent.click(screen.getByRole('button', { name: /save.*switch/i })); + + await screen.findByText(/verify the embeddings endpoint/i); + 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/app/test/playwright/specs/settings-leaf-workflows.spec.ts b/app/test/playwright/specs/settings-leaf-workflows.spec.ts index c04b79b1e..fe95023ce 100644 --- a/app/test/playwright/specs/settings-leaf-workflows.spec.ts +++ b/app/test/playwright/specs/settings-leaf-workflows.spec.ts @@ -105,7 +105,10 @@ test.describe('Settings leaf workflows', () => { await page .getByPlaceholder('https://your-endpoint.com/v1') .fill('http://127.0.0.1:18473/openai/v1'); - await page.getByPlaceholder('text-embedding-3-small').fill('e2e-embedding-model'); + // Use a `text-embedding-3-*` model so the save-time verification probe sends + // `dimensions: 64` — the mock backend (scripts/mock-api) echoes that length, + // so the live test embed verifies and the config can be persisted. + await page.getByPlaceholder('text-embedding-3-small').fill('text-embedding-3-small'); await page.getByPlaceholder('1024').fill('64'); await page.getByRole('button', { name: 'Save & switch' }).click(); @@ -127,7 +130,7 @@ test.describe('Settings leaf workflows', () => { }) .toEqual({ provider: 'custom:http://127.0.0.1:18473/openai/v1', - model: 'e2e-embedding-model', + model: 'text-embedding-3-small', dimensions: 64, }); }); diff --git a/scripts/mock-api/routes/integrations.mjs b/scripts/mock-api/routes/integrations.mjs index a640400e9..a1e5df08b 100644 --- a/scripts/mock-api/routes/integrations.mjs +++ b/scripts/mock-api/routes/integrations.mjs @@ -146,15 +146,31 @@ export function handleIntegrations(ctx) { const inputs = Array.isArray(parsedBody?.input) ? parsedBody.input : [parsedBody?.input ?? ""]; + // Honor the OpenAI `dimensions` parameter when present (the embeddings + // client sends it for `text-embedding-3-*` models). The save-time + // verification probe validates `vector.len() == configured dims`, so the + // returned vector must match the requested size; default to 4 for callers + // that don't request a specific size. + const requestedDims = + Number.isInteger(parsedBody?.dimensions) && parsedBody.dimensions > 0 + ? parsedBody.dimensions + : 4; const data = inputs.map((input, index) => { const text = String(input ?? ""); - // Keep the vector tiny but deterministic so callers that cache / - // compare embeddings can still observe stable output. + // Keep the vector deterministic so callers that cache / compare + // embeddings can still observe stable output: the first components keep + // the original `[basis, basis/10, basis/100, 1]` pattern, padded to the + // requested length. const basis = text.length || index + 1; + const seed = [basis, basis / 10, basis / 100, 1]; + const embedding = Array.from( + { length: requestedDims }, + (_, i) => seed[i] ?? 0, + ); return { object: "embedding", index, - embedding: [basis, basis / 10, basis / 100, 1], + embedding, }; }); json(res, 200, { @@ -334,11 +350,23 @@ export function handleIntegrations(ctx) { /^\/agent-integrations\/composio\/tools\/?(\?.*)?$/.test(url) ) { // Parse toolkits and tags from the query string. - const qs = url.includes("?") ? new URLSearchParams(url.split("?")[1]) : new URLSearchParams(); + const qs = url.includes("?") + ? new URLSearchParams(url.split("?")[1]) + : new URLSearchParams(); const toolkitsParam = qs.get("toolkits") ?? ""; const tagsParam = qs.get("tags") ?? ""; - const requestedToolkits = toolkitsParam ? toolkitsParam.split(",").map(t => t.trim().toLowerCase()).filter(Boolean) : []; - const requestedTags = tagsParam ? tagsParam.split(",").map(t => t.trim().toLowerCase()).filter(Boolean) : []; + const requestedToolkits = toolkitsParam + ? toolkitsParam + .split(",") + .map((t) => t.trim().toLowerCase()) + .filter(Boolean) + : []; + const requestedTags = tagsParam + ? tagsParam + .split(",") + .map((t) => t.trim().toLowerCase()) + .filter(Boolean) + : []; // Allow tests to inject per-tag tool lists via // composioToolsByTag_ (e.g. "composioToolsByTag_stars") @@ -373,9 +401,11 @@ export function handleIntegrations(ctx) { // Filter by toolkits when requested and the knob returns a list with a // "function.name" slug we can match (e.g. "GITHUB_*"). if (requestedToolkits.length > 0 && tools.length > 0) { - tools = tools.filter(t => { + tools = tools.filter((t) => { const name = (t?.function?.name ?? t?.name ?? "").toUpperCase(); - return requestedToolkits.some(tk => name.startsWith(tk.toUpperCase() + "_")); + return requestedToolkits.some((tk) => + name.startsWith(tk.toUpperCase() + "_"), + ); }); } } diff --git a/src/openhuman/embeddings/rpc.rs b/src/openhuman/embeddings/rpc.rs index b33ca9a29..615ddac0a 100644 --- a/src/openhuman/embeddings/rpc.rs +++ b/src/openhuman/embeddings/rpc.rs @@ -109,11 +109,17 @@ 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). + // Setup-time verification gate (TAURI-RUST-5JR / 4P4): a Custom + // (OpenAI-compatible) embeddings endpoint — e.g. LM Studio — must prove it + // can actually embed *before* we accept it. We run one live test embed and + // only persist the config if it succeeds; any failure (no `/embeddings` + // route, no model loaded, timeout, 5xx, empty/zero-dim vector) rejects the + // save so a config that can't embed is never stored (and we never wipe + // memory for one). Verifying at setup is the fix — we deliberately do NOT + // try to classify-and-suppress the resulting embed flood in code; any + // residual flood (e.g. the user unloads the model *after* a good save) is + // handled on the Sentry side. + // // 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 @@ -128,42 +134,33 @@ pub async fn update_settings( 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. + tracing::debug!( + provider = effective_provider.as_str(), + "{LOG_PREFIX} update_settings verifying embeddings endpoint with a test embed" + ); 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()], - )); - } + // Normalize the timeout/result into one shape, then apply the + // pure verification policy (`classify_embed_probe`, unit-tested). + let outcome = match probe { + Ok(Ok(vectors)) => EmbedProbe::Returned(vectors), + Ok(Err(e)) => EmbedProbe::Failed(e.to_string()), + Err(_elapsed) => EmbedProbe::TimedOut, + }; + if let Some(reject) = classify_embed_probe(outcome) { tracing::warn!( provider = effective_provider.as_str(), - error = detail.as_str(), - "{LOG_PREFIX} update_settings probe inconclusive — saving anyway" + "{LOG_PREFIX} update_settings rejected — embeddings endpoint failed verification" ); + return Ok(reject); } + tracing::debug!( + provider = effective_provider.as_str(), + "{LOG_PREFIX} update_settings test embed passed — accepting config" + ); } Err(e) => { // Construction failure (unknown slug / bad config) — surface it @@ -481,6 +478,97 @@ fn build_embedder( ) } +/// Normalized result of the setup-time test embed in [`update_settings`]. +/// Collapses the `Result, Elapsed>` timeout shape into one enum so +/// the verification policy can be expressed (and unit-tested) as a pure +/// function over it. +enum EmbedProbe { + /// The endpoint returned vectors (may still be empty/zero-dim — checked). + Returned(Vec>), + /// The embed call returned an error; the string is the provider detail. + Failed(String), + /// The probe didn't complete within the time box. + TimedOut, +} + +/// Setup-time embeddings verification policy. Returns `None` when the endpoint +/// is verified (accept + persist the config) or `Some(reject)` — the +/// "not saved" RPC payload — otherwise. +/// +/// The endpoint must prove it can embed before we accept it: only a non-empty +/// vector passes; every failure mode (no model loaded, no `/embeddings` route, +/// 5xx/auth/network, timeout, empty vector) rejects the save. We do NOT try to +/// classify-and-suppress the resulting embed flood in code — residual floods +/// (e.g. the user unloads the model after a good save) are handled Sentry-side. +/// The known shapes only get a friendlier remediation message. +fn classify_embed_probe(outcome: EmbedProbe) -> Option> { + let reject = |error: &str, message: &str, summary: &str, detail: Option<&str>| { + let mut body = serde_json::json!({ "error": error, "message": message }); + if let Some(d) = detail { + body["detail"] = serde_json::Value::String(d.to_string()); + } + Some(RpcOutcome::new(body, vec![summary.to_string()])) + }; + + match outcome { + // Pass only when the endpoint returns a usable vector. + EmbedProbe::Returned(vectors) + if vectors.first().map(|v| !v.is_empty()).unwrap_or(false) => + { + None + } + // Reachable but produced no usable vector — not a valid embedder. + EmbedProbe::Returned(_) => reject( + "EMBEDDINGS_VERIFICATION_FAILED", + "The embeddings endpoint responded but returned no vector. Choose an \ + embeddings-capable provider or endpoint, then save again.", + "test embed returned no vectors — not saved", + None, + ), + EmbedProbe::Failed(detail) => { + let lower = detail.to_ascii_lowercase(); + // Reachable but no model loaded (e.g. LM Studio idle). + if lower.contains("no models loaded") { + reject( + "EMBEDDINGS_NO_MODEL_LOADED", + "Your local embeddings server (e.g. LM Studio) is running but has no \ + model loaded. Load an embedding model — in LM Studio use the developer \ + page or the `lms load` command — then save again.", + "embeddings server has no model loaded — not saved", + Some(&detail), + ) + } else if crate::core::observability::is_embedding_endpoint_absent(&lower) { + // Endpoint exposes no embeddings API (404/405). + reject( + "EMBEDDINGS_ENDPOINT_NO_API", + "This endpoint has no embeddings API. Choose an embeddings-capable \ + provider (Managed, Voyage, OpenAI, Cohere, Ollama) or a different \ + custom endpoint.", + "embeddings endpoint has no embeddings API — not saved", + Some(&detail), + ) + } else { + // Any other failure (5xx, auth, network) — didn't pass verification. + reject( + "EMBEDDINGS_VERIFICATION_FAILED", + "Couldn't verify the embeddings endpoint — the test embed failed. Make \ + sure the endpoint is reachable and serving an embedding model, then \ + save again.", + "embeddings endpoint failed verification — not saved", + Some(&detail), + ) + } + } + EmbedProbe::TimedOut => reject( + "EMBEDDINGS_VERIFICATION_FAILED", + "Couldn't verify the embeddings endpoint — the test embed timed out. Make sure \ + the endpoint is running and reachable, then save again.", + "embeddings endpoint timed out during verification — not saved", + None, + ), + } +} + pub(crate) fn resolve_api_key(config: &Config, provider_name: &str) -> String { let slug = if provider_name.starts_with("custom:") { "custom" @@ -557,4 +645,87 @@ mod tests { "sk-custom-test" ); } + + /// Helper: pull the `error` code out of a reject payload. + fn reject_code(outcome: EmbedProbe) -> Option { + classify_embed_probe(outcome).map(|rpc| { + rpc.value + .get("error") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string() + }) + } + + /// A usable vector is the ONLY thing that passes the setup-time gate — the + /// config is then accepted and persisted. + #[test] + fn classify_embed_probe_accepts_only_usable_vector() { + assert!( + classify_embed_probe(EmbedProbe::Returned(vec![vec![0.1, 0.2, 0.3]])).is_none(), + "a non-empty vector must verify the endpoint" + ); + } + + /// Reachable but empty/zero-dim response is a failed verification, not a + /// valid embedder — never persist it. + #[test] + fn classify_embed_probe_rejects_empty_vectors() { + assert_eq!( + reject_code(EmbedProbe::Returned(vec![])).as_deref(), + Some("EMBEDDINGS_VERIFICATION_FAILED") + ); + assert_eq!( + reject_code(EmbedProbe::Returned(vec![vec![]])).as_deref(), + Some("EMBEDDINGS_VERIFICATION_FAILED") + ); + } + + /// LM Studio idle ("No models loaded") must reject the save with the + /// one-step remediation code so the doomed config is never persisted — the + /// fix is verifying at setup, not suppressing the later flood. + #[test] + fn classify_embed_probe_rejects_no_model_loaded() { + let body = r#"Embedding API error (400 Bad Request): {"error":"No models loaded. Please load a model in the developer page or use the 'lms load' command."}"#; + let rpc = classify_embed_probe(EmbedProbe::Failed(body.to_string())).unwrap(); + assert_eq!( + rpc.value.get("error").and_then(|v| v.as_str()), + Some("EMBEDDINGS_NO_MODEL_LOADED") + ); + // The raw provider detail is preserved for the UI. + assert_eq!(rpc.value.get("detail").and_then(|v| v.as_str()), Some(body)); + } + + /// A 404/405 (no `/embeddings` route) keeps its dedicated code. + #[test] + fn classify_embed_probe_rejects_endpoint_absent() { + assert_eq!( + reject_code(EmbedProbe::Failed( + "Embedding API error (404 Not Found): no route".into() + )) + .as_deref(), + Some("EMBEDDINGS_ENDPOINT_NO_API") + ); + } + + /// Any other failure (5xx/auth/network) and timeouts both reject — the + /// endpoint didn't prove it can embed, so we don't accept it. + #[test] + fn classify_embed_probe_rejects_other_failures_and_timeout() { + assert_eq!( + reject_code(EmbedProbe::Failed( + "Embedding API error (500 Internal Server Error): boom".into() + )) + .as_deref(), + Some("EMBEDDINGS_VERIFICATION_FAILED") + ); + assert_eq!( + reject_code(EmbedProbe::Failed("connection refused".into())).as_deref(), + Some("EMBEDDINGS_VERIFICATION_FAILED") + ); + assert_eq!( + reject_code(EmbedProbe::TimedOut).as_deref(), + Some("EMBEDDINGS_VERIFICATION_FAILED") + ); + } } diff --git a/tests/embeddings_rpc_e2e.rs b/tests/embeddings_rpc_e2e.rs index 3a840b743..fb26c2a48 100644 --- a/tests/embeddings_rpc_e2e.rs +++ b/tests/embeddings_rpc_e2e.rs @@ -130,6 +130,16 @@ async fn mock_openai_embeddings( headers: axum::http::HeaderMap, Json(body): Json, ) -> Json { + // Return exactly one embedding per input, like a real OpenAI-compatible + // server — the client rejects a count mismatch (`openai embed count + // mismatch`), and the save-time verification probe sends a single + // `"connection test"` input while the real embed sends two. Captured before + // `body` is moved into the request log below. + let input_len = match body.get("input") { + Some(Value::Array(items)) => items.len().max(1), + _ => 1, + }; + state .requests .lock() @@ -145,12 +155,23 @@ async fn mock_openai_embeddings( .and_then(|value| value.to_str().ok()) .map(ToOwned::to_owned), ); + + // Index 0 → [0.1,0.2,0.3], every other index → [0.4,0.5,0.6], so the + // two-input embed assertion (`vectors[1][2] ≈ 0.6`) still holds. + let data: Vec = (0..input_len) + .map(|i| { + let embedding = if i == 0 { + [0.1, 0.2, 0.3] + } else { + [0.4, 0.5, 0.6] + }; + json!({ "object": "embedding", "index": i, "embedding": embedding }) + }) + .collect(); + Json(json!({ "object": "list", - "data": [ - { "object": "embedding", "index": 0, "embedding": [0.1, 0.2, 0.3] }, - { "object": "embedding", "index": 1, "embedding": [0.4, 0.5, 0.6] } - ], + "data": data, "model": "mock-embedding-model" })) }