From ae7dd39927bd3b5289bc18bbe03d6a1f35dc78b3 Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Wed, 24 Jun 2026 00:23:11 +0530 Subject: [PATCH] fix(inference): gate /responses fallback on provider capability (#3997) (#4009) --- .../config/schema/cloud_providers.rs | 76 ++++++++++- src/openhuman/inference/provider/factory.rs | 42 ++++-- .../inference/provider/factory_tests.rs | 124 ++++++++++++++++++ 3 files changed, 231 insertions(+), 11 deletions(-) diff --git a/src/openhuman/config/schema/cloud_providers.rs b/src/openhuman/config/schema/cloud_providers.rs index 1170d3146..db0b104c2 100644 --- a/src/openhuman/config/schema/cloud_providers.rs +++ b/src/openhuman/config/schema/cloud_providers.rs @@ -199,6 +199,36 @@ fn builtin_cloud_provider(type_str: &str) -> Option<&'static BuiltinCloudProvide .find(|provider| provider.slug == type_str) } +/// Whether `slug` matches a built-in cloud provider preset. +/// +/// The chat factory uses this to decide capability defaults (e.g. whether the +/// provider exposes the OpenAI Responses API) only for providers we ship and +/// therefore know the API surface of. Custom / user-defined slugs are treated +/// as unknown and keep the permissive defaults. +pub fn is_builtin_cloud_slug(slug: &str) -> bool { + builtin_cloud_provider(slug).is_some() +} + +/// Whether a built-in cloud provider exposes the OpenAI **Responses API** +/// (`/v1/responses`). +/// +/// Only OpenAI's first-party endpoint serves `/responses`; every other built-in +/// preset (DeepSeek, Groq, Mistral, Fireworks, …) is chat-completions-only. +/// Enabling the chat-completions-404 → `/responses` fallback for those +/// guarantees a second 404 against an endpoint that does not exist, which floods +/// Sentry with an empty-body `" Responses API error:"` event +/// (TAURI-RUST-5EN — same class as the local-provider TAURI-RUST-59Y fix). The +/// factory consults this to build chat-completions-only built-ins with +/// `new_no_responses_fallback`. +/// +/// Custom / unknown slugs are intentionally NOT covered here (see +/// [`is_builtin_cloud_slug`]): a user-defined OpenAI-compatible endpoint may be +/// a genuine OpenAI proxy that does support `/responses`, so the factory keeps +/// the fallback for those. +pub fn builtin_cloud_supports_responses_api(slug: &str) -> bool { + matches!(slug, "openai") +} + /// Authentication header style for a cloud provider. /// /// Wire format is lowercase (e.g. `"bearer"`). Determines which HTTP headers @@ -474,8 +504,8 @@ impl CloudProviderType { #[cfg(test)] mod tests { use super::{ - is_slug_reserved, migrate_legacy_fields, AuthStyle, CloudProviderCreds, - BUILTIN_CLOUD_PROVIDERS, + builtin_cloud_supports_responses_api, is_builtin_cloud_slug, is_slug_reserved, + migrate_legacy_fields, AuthStyle, CloudProviderCreds, BUILTIN_CLOUD_PROVIDERS, }; #[test] @@ -562,4 +592,46 @@ mod tests { ); } } + + #[test] + fn is_builtin_cloud_slug_matches_presets_only() { + for slug in ["openai", "deepseek", "groq", "mistral"] { + assert!(is_builtin_cloud_slug(slug), "{slug} is a built-in preset"); + } + for slug in ["my-proxy", "custom-openai", "totally-unknown", ""] { + assert!( + !is_builtin_cloud_slug(slug), + "{slug:?} is not a built-in preset" + ); + } + } + + #[test] + fn only_openai_builtin_exposes_responses_api() { + assert!(builtin_cloud_supports_responses_api("openai")); + for slug in ["deepseek", "groq", "mistral", "fireworks", "together"] { + assert!( + !builtin_cloud_supports_responses_api(slug), + "{slug} is chat-completions-only and must not advertise the Responses API" + ); + } + } + + /// Drift guard (TAURI-RUST-5EN): couple the capability helper to the + /// preset list so adding a new built-in that wrongly claims the Responses + /// API — or renaming `openai` — fails CI rather than silently re-enabling + /// the guaranteed-404 `/responses` fallback. OpenAI's first-party endpoint + /// is the only built-in that serves `/v1/responses`. + #[test] + fn responses_api_capability_is_coupled_to_the_preset_list() { + for provider in BUILTIN_CLOUD_PROVIDERS { + let expected = provider.slug == "openai"; + assert_eq!( + builtin_cloud_supports_responses_api(provider.slug), + expected, + "built-in {} Responses-API capability drifted from the openai-only invariant", + provider.slug + ); + } + } } diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 979c66230..0e595eaf7 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -24,7 +24,9 @@ //! //! Unknown slugs and missing-creds configurations produce actionable errors. -use crate::openhuman::config::schema::cloud_providers::AuthStyle; +use crate::openhuman::config::schema::cloud_providers::{ + builtin_cloud_supports_responses_api, is_builtin_cloud_slug, AuthStyle, +}; use crate::openhuman::config::Config; use crate::openhuman::credentials::AuthService; use crate::openhuman::inference::provider::claude_agent_sdk::subprocess::ClaudeAgentSdkProvider; @@ -1512,14 +1514,36 @@ fn make_cloud_provider_by_slug( redact_endpoint(&openai_codex_routing.endpoint), openai_codex_routing.account_id.is_some() ); - let mut provider = OpenAiCompatibleProvider::new( - slug, - &openai_codex_routing.endpoint, - (!key.trim().is_empty()).then_some(key.as_str()), - CompatAuthStyle::Bearer, - ) - .with_temperature_unsupported_models(unsupported.to_vec()) - .with_temperature_override(temperature_override); + // Enable the chat-completions-404 → `/v1/responses` fallback only + // for providers that actually expose the Responses API. Built-in + // chat-completions-only providers (DeepSeek, Groq, Mistral, …) do + // not — hitting their non-existent `/responses` guarantees a second + // 404 and floods Sentry with an empty-body " Responses + // API error:" event (TAURI-RUST-5EN, same class as the + // local-provider TAURI-RUST-59Y fix). OpenAI keeps the fallback + // (genuine `/responses`), and so do custom / unknown slugs, whose + // endpoint may be a real OpenAI proxy. + let responses_fallback = + !is_builtin_cloud_slug(slug) || builtin_cloud_supports_responses_api(slug); + let credential = (!key.trim().is_empty()).then_some(key.as_str()); + let base_provider = if responses_fallback { + OpenAiCompatibleProvider::new( + slug, + &openai_codex_routing.endpoint, + credential, + CompatAuthStyle::Bearer, + ) + } else { + OpenAiCompatibleProvider::new_no_responses_fallback( + slug, + &openai_codex_routing.endpoint, + credential, + CompatAuthStyle::Bearer, + ) + }; + let mut provider = base_provider + .with_temperature_unsupported_models(unsupported.to_vec()) + .with_temperature_override(temperature_override); if let Some(account_id) = openai_codex_routing.account_id.as_deref() { provider = provider.with_extra_header(OPENAI_CODEX_ACCOUNT_HEADER, account_id); } diff --git a/src/openhuman/inference/provider/factory_tests.rs b/src/openhuman/inference/provider/factory_tests.rs index dc14fea2c..eb602622f 100644 --- a/src/openhuman/inference/provider/factory_tests.rs +++ b/src/openhuman/inference/provider/factory_tests.rs @@ -1536,6 +1536,130 @@ async fn cloud_provider_falls_back_to_responses_on_404() { drop(result); } +/// TAURI-RUST-5EN: a built-in chat-completions-only cloud provider (DeepSeek) +/// must NOT fall back to `/v1/responses` on a chat-completions 404. DeepSeek +/// exposes no Responses API, so the fallback is a guaranteed second 404 that +/// floods Sentry with an empty-body "deepseek Responses API error:" event. +/// Bearer-path counterpart to `ollama_provider_does_not_fall_back_to_responses_on_404`. +#[tokio::test] +async fn deepseek_builtin_does_not_fall_back_to_responses_on_404() { + let mock_server = MockServer::start().await; + + Mock::given(method("POST")) + .and(path("/v1/chat/completions")) + .respond_with( + ResponseTemplate::new(404) + .set_body_string(r#"{"error":{"message":"model not found","code":404}}"#), + ) + .expect(1) // exactly one attempt — no retry, no fallback + .mount(&mock_server) + .await; + + // DeepSeek has no /v1/responses — the fallback must never reach it. + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(ResponseTemplate::new(404).set_body_string("")) + .expect(0) // must not be called + .mount(&mock_server) + .await; + + let tmp = TempDir::new().expect("tempdir"); + let entry = CloudProviderCreds { + id: "p_deepseek".to_string(), + slug: "deepseek".to_string(), + label: "DeepSeek".to_string(), + endpoint: format!("{}/v1", mock_server.uri()), + auth_style: AuthStyle::Bearer, + default_model: Some("deepseek-v4-flash".to_string()), + ..Default::default() + }; + let config = config_with_providers_in_tempdir(&tmp, vec![entry]); + // Bearer providers fail at call time with "API key not set" before any HTTP + // request, so stash a key to let the chat-completions call reach the mock. + AuthService::from_config(&config) + .store_provider_token( + "provider:deepseek", + "default", + "sk-test", + Default::default(), + true, + ) + .expect("store provider token"); + + let (provider, model) = + create_chat_provider_from_string("chat", "deepseek:deepseek-v4-flash", &config) + .expect("deepseek provider must build"); + + let result = provider.chat_with_system(None, "hello", &model, 0.0).await; + assert!( + result.is_err(), + "chat-completions 404 should surface as an error, not a success" + ); + + // wiremock verifies expect(0) on /v1/responses when the server is dropped. +} + +/// Counterpart guard: a custom (non-built-in) Bearer slug KEEPS the responses +/// fallback — its endpoint may be a genuine OpenAI proxy that serves +/// `/v1/responses`. Ensures the 5EN slug-gate only disables the fallback for +/// known chat-completions-only built-ins, not for unknown providers. +#[tokio::test] +async fn custom_bearer_provider_keeps_responses_fallback_on_404() { + let mock_server = MockServer::start().await; + + Mock::given(method("POST")) + .and(path("/v1/chat/completions")) + .respond_with( + ResponseTemplate::new(404) + .set_body_string(r#"{"error":{"message":"model not found","code":404}}"#), + ) + .expect(1) + .mount(&mock_server) + .await; + + // Unknown slug → fallback retained → /v1/responses MUST be called. + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with( + ResponseTemplate::new(200).set_body_string( + r#"{"output":[{"content":[{"type":"output_text","text":"ok"}]}]}"#, + ), + ) + .expect(1) + .mount(&mock_server) + .await; + + let tmp = TempDir::new().expect("tempdir"); + let entry = CloudProviderCreds { + id: "p_custom".to_string(), + slug: "my-openai-proxy".to_string(), + label: "My Proxy".to_string(), + endpoint: format!("{}/v1", mock_server.uri()), + auth_style: AuthStyle::Bearer, + default_model: Some("proxy-model".to_string()), + ..Default::default() + }; + let config = config_with_providers_in_tempdir(&tmp, vec![entry]); + AuthService::from_config(&config) + .store_provider_token( + "provider:my-openai-proxy", + "default", + "sk-test", + Default::default(), + true, + ) + .expect("store provider token"); + + let (provider, model) = + create_chat_provider_from_string("chat", "my-openai-proxy:proxy-model", &config) + .expect("custom bearer provider must build"); + + let result = provider.chat_with_system(None, "hello", &model, 0.0).await; + drop(result); + + // wiremock verifies expect(1) on /v1/responses when the server is dropped. +} + #[tokio::test] #[ignore = "requires live LM Studio on localhost:1234"] async fn live_lmstudio_provider_streams_thinking_and_text() {