diff --git a/src/openhuman/inference/provider/compatible_helpers.rs b/src/openhuman/inference/provider/compatible_helpers.rs index 11d7fc6ec..801c67c52 100644 --- a/src/openhuman/inference/provider/compatible_helpers.rs +++ b/src/openhuman/inference/provider/compatible_helpers.rs @@ -53,147 +53,189 @@ impl OpenAiCompatibleProvider { }) .unwrap_or(false); - let request = ResponsesRequest { + // TAURI-RUST-EWD: the Codex/ChatGPT OAuth Responses endpoint + // (`chatgpt.com/backend-api/codex/responses`) rejects `max_output_tokens` + // outright (400 `Unsupported parameter: max_output_tokens`), the same way + // it rejects `stream: false` (#3201). Omit the cap proactively for that + // endpoint; every other Responses backend keeps it (the cap still flows + // through for `responses_api_primary` on a real `/v1/responses`). + if is_codex_oauth_responses && max_output_tokens.is_some() { + log::debug!( + "[provider] {} omitting max_output_tokens={:?} for Codex OAuth responses endpoint (unsupported param)", + self.name, + max_output_tokens, + ); + } + let mut request = ResponsesRequest { model: model.to_string(), input, instructions, stream: Some(is_codex_oauth_responses), store: Some(false), - max_output_tokens, + max_output_tokens: if is_codex_oauth_responses { + None + } else { + max_output_tokens + }, }; let url = self.responses_url(); + let mut retried_without_cap = false; - let response = self - .apply_auth_header(self.http_client().post(&url).json(&request), credential) - .send() - .await?; + let (status, error) = loop { + let response = self + .apply_auth_header(self.http_client().post(&url).json(&request), credential) + .send() + .await?; + + if response.status().is_success() { + let body = response.text().await?; + if is_codex_oauth_responses { + return aggregate_responses_sse_body(&self.name, &body); + } + let responses = parse_responses_response_body(&self.name, &body)?; + return extract_responses_text(responses).ok_or_else(|| { + anyhow::anyhow!("No response from {} Responses API", self.name) + }); + } - if !response.status().is_success() { let status = response.status(); - let status_str = status.as_u16().to_string(); let error = response.text().await?; - let sanitized = super::super::sanitize_api_error(&error); - // Emit the status in the structured `()` position the retry - // classifier understands (`reliable::structured_http_4xx`). The bare - // `"… Responses API error: 404 Not Found"` form left the `404` - // unanchored, so a terminal 404 was misclassified as retryable and - // looped indefinitely (TAURI-RUST-FJZ, ~15k events). - let message = format!( - "{} Responses API error ({status_str}): {sanitized}", - self.name - ); - // A 404 from the `/responses` route can mean this endpoint has no - // Responses API at all — disable the chat-completions-404 → - // `/responses` fallback for it so we stop issuing a guaranteed second - // 404. Guard against poisoning the process-global cache on a - // model/deployment-specific 404 (the route exists, the model - // doesn't), which would wrongly drop the fallback for every other - // model on a Responses-capable endpoint. Skip when Responses is the - // primary path (Codex OAuth): the fallback flag is never consulted - // and a 404 there is not evidence the route is missing. - if status == reqwest::StatusCode::NOT_FOUND - && !self.responses_api_primary - && Self::responses_404_indicates_missing_route(&error) + + // Reactive defense-in-depth: a strict Responses backend may still + // reject `max_output_tokens` with a 400 (e.g. a future Codex endpoint + // variant we don't match above). Strip the field and retry once, + // mirroring the no-tools / frequency_penalty retries. Bounded to a + // single retry so a genuinely different 400 still surfaces. + if !retried_without_cap + && request.max_output_tokens.is_some() + && status == reqwest::StatusCode::BAD_REQUEST + && Self::err_indicates_max_output_tokens_unsupported(&error) { - super::mark_responses_api_unsupported(&self.base_url); + log::info!( + "[provider] {} rejected max_output_tokens — retrying responses request without it", + self.name, + ); + request.max_output_tokens = None; + retried_without_cap = true; + continue; } - if super::super::is_budget_exhausted_http_400(status, &error) { - super::super::log_budget_exhausted_http_400( - "responses_api", - self.name.as_str(), - Some(model), - status, - ); - } else if super::super::is_custom_openai_upstream_bad_request_http_400( - self.name.as_str(), - status, - &error, - ) { - super::super::log_custom_openai_upstream_bad_request_http_400( - "responses_api", - self.name.as_str(), - Some(model), - status, - ); - } else if super::super::is_provider_access_policy_denied_http_403(status, &error) { - super::super::log_provider_access_policy_denied_http_403( - "responses_api", - self.name.as_str(), - Some(model), - status, - ); - } else if super::super::is_provider_config_rejection_http( - status, - self.name.as_str(), - &error, - ) { - super::super::log_provider_config_rejection( - "responses_api", - self.name.as_str(), - Some(model), - status, - ); - } else if super::super::is_byo_provider_auth_failure_http( - self.name.as_str(), - status, - &error, - ) { - super::super::log_byo_provider_auth_failure( - "responses_api", - self.name.as_str(), - Some(model), - status, - ); - } else if super::super::is_openai_oauth_session_expired_http( - self.name.as_str(), - status, - &error, - ) { - super::super::log_openai_oauth_session_expired( - "responses_api", - self.name.as_str(), - Some(model), - status, - ); - } else if super::super::is_provider_insufficient_credits_402(status, &error) { - // Insufficient-credits 402: the user's own BYO provider account - // is out of balance — a flat billing fact, not a reservation- - // window error, so there is NO local max_tokens lever to apply. - // Demote to info instead of paging on every retry; this is the - // complete classification for a genuinely-unpreventable - // BYO-balance condition - // (TAURI-RUST-4QF — DeepSeek "Insufficient Balance"). - super::super::log_provider_insufficient_credits_402( - "responses_api", - self.name.as_str(), - Some(model), - status, - ); - } else if super::super::should_report_provider_http_failure(status) { - crate::core::observability::report_error( - message.as_str(), - "llm_provider", - "responses_api", - &[ - ("provider", self.name.as_str()), - ("model", model), - ("status", status_str.as_str()), - ("failure", "non_2xx"), - ], - ); - } - anyhow::bail!(message); - } - let body = response.text().await?; - if is_codex_oauth_responses { - return aggregate_responses_sse_body(&self.name, &body); - } - let responses = parse_responses_response_body(&self.name, &body)?; + break (status, error); + }; - extract_responses_text(responses) - .ok_or_else(|| anyhow::anyhow!("No response from {} Responses API", self.name)) + let status_str = status.as_u16().to_string(); + let sanitized = super::super::sanitize_api_error(&error); + // Emit the status in the structured `()` position the retry + // classifier understands (`reliable::structured_http_4xx`). The bare + // `"… Responses API error: 404 Not Found"` form left the `404` + // unanchored, so a terminal 404 was misclassified as retryable and + // looped indefinitely (TAURI-RUST-FJZ, ~15k events). + let message = format!( + "{} Responses API error ({status_str}): {sanitized}", + self.name + ); + // A 404 from the `/responses` route can mean this endpoint has no + // Responses API at all — disable the chat-completions-404 → + // `/responses` fallback for it so we stop issuing a guaranteed second + // 404. Guard against poisoning the process-global cache on a + // model/deployment-specific 404 (the route exists, the model + // doesn't), which would wrongly drop the fallback for every other + // model on a Responses-capable endpoint. Skip when Responses is the + // primary path (Codex OAuth): the fallback flag is never consulted + // and a 404 there is not evidence the route is missing. + if status == reqwest::StatusCode::NOT_FOUND + && !self.responses_api_primary + && Self::responses_404_indicates_missing_route(&error) + { + super::mark_responses_api_unsupported(&self.base_url); + } + if super::super::is_budget_exhausted_http_400(status, &error) { + super::super::log_budget_exhausted_http_400( + "responses_api", + self.name.as_str(), + Some(model), + status, + ); + } else if super::super::is_custom_openai_upstream_bad_request_http_400( + self.name.as_str(), + status, + &error, + ) { + super::super::log_custom_openai_upstream_bad_request_http_400( + "responses_api", + self.name.as_str(), + Some(model), + status, + ); + } else if super::super::is_provider_access_policy_denied_http_403(status, &error) { + super::super::log_provider_access_policy_denied_http_403( + "responses_api", + self.name.as_str(), + Some(model), + status, + ); + } else if super::super::is_provider_config_rejection_http( + status, + self.name.as_str(), + &error, + ) { + super::super::log_provider_config_rejection( + "responses_api", + self.name.as_str(), + Some(model), + status, + ); + } else if super::super::is_byo_provider_auth_failure_http( + self.name.as_str(), + status, + &error, + ) { + super::super::log_byo_provider_auth_failure( + "responses_api", + self.name.as_str(), + Some(model), + status, + ); + } else if super::super::is_openai_oauth_session_expired_http( + self.name.as_str(), + status, + &error, + ) { + super::super::log_openai_oauth_session_expired( + "responses_api", + self.name.as_str(), + Some(model), + status, + ); + } else if super::super::is_provider_insufficient_credits_402(status, &error) { + // Insufficient-credits 402: the user's own BYO provider account + // is out of balance — a flat billing fact, not a reservation- + // window error, so there is NO local max_tokens lever to apply. + // Demote to info instead of paging on every retry; this is the + // complete classification for a genuinely-unpreventable + // BYO-balance condition + // (TAURI-RUST-4QF — DeepSeek "Insufficient Balance"). + super::super::log_provider_insufficient_credits_402( + "responses_api", + self.name.as_str(), + Some(model), + status, + ); + } else if super::super::should_report_provider_http_failure(status) { + crate::core::observability::report_error( + message.as_str(), + "llm_provider", + "responses_api", + &[ + ("provider", self.name.as_str()), + ("model", model), + ("status", status_str.as_str()), + ("failure", "non_2xx"), + ], + ); + } + anyhow::bail!(message); } pub(super) fn convert_tool_specs( @@ -624,6 +666,29 @@ impl OpenAiCompatibleProvider { || lower.contains("unexpected")) } + /// Detect a Responses backend rejecting the `max_output_tokens` field as an + /// *unrecognized parameter*. The Codex/ChatGPT OAuth endpoint 400s with + /// `Unsupported parameter: max_output_tokens` (TAURI-RUST-EWD); when this + /// fires the responses path retries once with the field omitted (mirrors the + /// no-tools and frequency_penalty retries). String-based because the + /// rejection surfaces as the API error body. + /// + /// Deliberately matches only *parameter-not-accepted* wording, **not** + /// invalid-value wording (e.g. "value above the allowed range"). Stripping + /// the cap turns a bounded request into an uncapped generation, so on a + /// value-range error we must surface the config error rather than silently + /// drop the credit-preflight / response-size protection `max_tokens` gives. + pub(super) fn err_indicates_max_output_tokens_unsupported(error: &str) -> bool { + let lower = error.to_lowercase(); + lower.contains("max_output_tokens") + && (lower.contains("unsupported") + || lower.contains("unknown") + || lower.contains("unrecognized") + || lower.contains("not supported") + || lower.contains("does not support") + || lower.contains("unexpected parameter")) + } + /// Disambiguate a 404 from the `/responses` route: `true` when it signals the /// *route itself* is absent (this endpoint has no Responses API), `false` when /// it looks model/deployment-specific (the route exists, that model doesn't). diff --git a/src/openhuman/inference/provider/compatible_tests.rs b/src/openhuman/inference/provider/compatible_tests.rs index 3973af2e5..ea89ca667 100644 --- a/src/openhuman/inference/provider/compatible_tests.rs +++ b/src/openhuman/inference/provider/compatible_tests.rs @@ -885,6 +885,191 @@ async fn responses_api_primary_posts_directly_to_responses() { assert_eq!(text, "hello from responses"); } +/// TAURI-RUST-EWD: the Codex/ChatGPT OAuth Responses endpoint rejects +/// `max_output_tokens` (400 `Unsupported parameter: max_output_tokens`). The +/// memory-extraction cap (`ChatRequest::max_tokens`) must therefore be dropped +/// on the wire for `/backend-api/codex/responses`. The exact `body_json` +/// matcher omits the field, so a request that still carried it would fail to +/// match the mock and the call would error — i.e. a match positively proves the +/// omission. +#[tokio::test] +async fn codex_responses_omits_max_output_tokens() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/backend-api/codex/responses")) + .and(body_json(serde_json::json!({ + "model": "gpt-5.5", + "input": [{ + "role": "user", + "content": [{"type": "input_text", "text": "hello"}] + }], + "stream": true, + "store": false + }))) + .respond_with(ResponseTemplate::new(200).set_body_string( + "data: {\"type\":\"response.output_text.delta\",\"delta\":\"ok\"}\n\n\ + data: {\"type\":\"response.completed\",\"response\":{\"output_text\":\"ok\"}}\n\n\ + data: [DONE]\n\n", + )) + .mount(&server) + .await; + + let provider = OpenAiCompatibleProvider::new( + "openai", + &format!("{}/backend-api/codex", server.uri()), + Some("oauth-access-token"), + AuthStyle::Bearer, + ) + .with_responses_api_primary(); + + let messages = [ChatMessage::user("hello")]; + let request = crate::openhuman::inference::provider::traits::ChatRequest { + messages: &messages, + tools: None, + stream: None, + // A bounded cap (memory extraction) that the Codex endpoint rejects. + max_tokens: Some(256), + }; + let resp = provider.chat(request, "gpt-5.5", 0.0).await.unwrap(); + assert_eq!(resp.text.as_deref(), Some("ok")); +} + +/// A non-Codex Responses backend (`/v1/responses`) must still receive the cap — +/// the omission is endpoint-specific, not a blanket drop (guards against +/// silently uncapping memory extraction on real `/v1/responses`, TAURI-RUST-C62). +#[tokio::test] +async fn standard_responses_keeps_max_output_tokens() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v1/responses")) + .and(body_json(serde_json::json!({ + "model": "gpt-5.4", + "input": [{ + "role": "user", + "content": [{"type": "input_text", "text": "hello"}] + }], + "stream": false, + "store": false, + "max_output_tokens": 256 + }))) + .respond_with( + ResponseTemplate::new(200).set_body_string(r#"{"output_text":"ok","output":[]}"#), + ) + .mount(&server) + .await; + + let provider = OpenAiCompatibleProvider::new( + "openai", + &format!("{}/v1", server.uri()), + Some("sk-test"), + AuthStyle::Bearer, + ) + .with_responses_api_primary(); + + let messages = [ChatMessage::user("hello")]; + let request = crate::openhuman::inference::provider::traits::ChatRequest { + messages: &messages, + tools: None, + stream: None, + max_tokens: Some(256), + }; + let resp = provider.chat(request, "gpt-5.4", 0.0).await.unwrap(); + assert_eq!(resp.text.as_deref(), Some("ok")); +} + +/// The reactive strip-and-retry defends Responses backends we don't match by +/// URL: a 400 whose body flags `max_output_tokens` as unsupported triggers one +/// retry with the field removed, which then succeeds. Exact-body matchers are +/// mutually exclusive on `max_output_tokens`, so the result is deterministic +/// regardless of mock precedence. +#[tokio::test] +async fn responses_strips_max_output_tokens_and_retries_on_400() { + let server = MockServer::start().await; + let input = serde_json::json!([{ + "role": "user", + "content": [{"type": "input_text", "text": "hello"}] + }]); + Mock::given(method("POST")) + .and(path("/v1/responses")) + .and(body_json(serde_json::json!({ + "model": "gpt-5.5", + "input": input.clone(), + "stream": false, + "store": false, + "max_output_tokens": 256 + }))) + .respond_with( + ResponseTemplate::new(400) + .set_body_string(r#"{"detail":"Unsupported parameter: max_output_tokens"}"#), + ) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/v1/responses")) + .and(body_json(serde_json::json!({ + "model": "gpt-5.5", + "input": input.clone(), + "stream": false, + "store": false + }))) + .respond_with( + ResponseTemplate::new(200) + .set_body_string(r#"{"output_text":"recovered","output":[]}"#), + ) + .mount(&server) + .await; + + let provider = OpenAiCompatibleProvider::new( + "strict-responses", + &format!("{}/v1", server.uri()), + Some("sk-test"), + AuthStyle::Bearer, + ) + .with_responses_api_primary(); + + let messages = [ChatMessage::user("hello")]; + let request = crate::openhuman::inference::provider::traits::ChatRequest { + messages: &messages, + tools: None, + stream: None, + max_tokens: Some(256), + }; + let resp = provider.chat(request, "gpt-5.5", 0.0).await.unwrap(); + assert_eq!(resp.text.as_deref(), Some("recovered")); +} + +#[test] +fn err_indicates_max_output_tokens_unsupported_matches_codex_body() { + assert!( + OpenAiCompatibleProvider::err_indicates_max_output_tokens_unsupported( + r#"{"detail":"Unsupported parameter: max_output_tokens"}"# + ) + ); + assert!( + !OpenAiCompatibleProvider::err_indicates_max_output_tokens_unsupported( + r#"{"detail":"rate limit exceeded"}"# + ) + ); + // A different unsupported parameter must not trip the max_output_tokens strip. + assert!( + !OpenAiCompatibleProvider::err_indicates_max_output_tokens_unsupported( + r#"{"detail":"Unsupported parameter: temperature"}"# + ) + ); + // An invalid-*value* error (cap out of the model's allowed range) must NOT + // strip the cap — that would silently uncap the request. Surface it instead. + assert!( + !OpenAiCompatibleProvider::err_indicates_max_output_tokens_unsupported( + r#"{"detail":"Invalid value for max_output_tokens: must be <= 4096"}"# + ) + ); + assert!( + !OpenAiCompatibleProvider::err_indicates_max_output_tokens_unsupported( + r#"{"detail":"max_output_tokens exceeds the maximum allowed for this model"}"# + ) + ); +} + #[test] fn blank_required_key_counts_as_missing() { let p = OpenAiCompatibleProvider::new(