diff --git a/src/openhuman/agent_orchestration/tools/dispatch.rs b/src/openhuman/agent_orchestration/tools/dispatch.rs index 212199c48..5d9b2030e 100644 --- a/src/openhuman/agent_orchestration/tools/dispatch.rs +++ b/src/openhuman/agent_orchestration/tools/dispatch.rs @@ -119,11 +119,30 @@ pub(crate) async fn dispatch_subagent( agent_id: definition.id.clone(), error: message.clone(), }); - Ok(ToolResult::error(format!("{tool_name} failed: {message}"))) + // Make the failure unmistakable to the orchestrator: the delegated + // task did NOT run, so it must not be reported as success or have + // its output fabricated. Without this guardrail a weak orchestrator + // can narrate a plausible success from the bare error text — the + // "hallucinated success" half of #3193 (e.g. claiming `run_code` + // wrote a file when the coding model 404'd and nothing executed). + Ok(ToolResult::error(format_subagent_failure( + tool_name, &message, + ))) } } } +/// Format a subagent-delegation failure so the orchestrator cannot mistake it +/// for success. Kept as a standalone, side-effect-free fn so the exact wording +/// is unit-testable without standing up a registry + failing model (#3193). +fn format_subagent_failure(tool_name: &str, message: &str) -> String { + format!( + "{tool_name} failed and did not complete — no work was performed and no \ + results were produced. Do NOT treat this as success or fabricate an \ + output; report the failure to the user. Error: {message}" + ) +} + #[cfg(test)] mod tests { use super::*; @@ -162,4 +181,31 @@ mod tests { "unexpected graceful-failure message: {out}" ); } + + #[test] + fn subagent_failure_envelope_forbids_fabricated_success() { + // #3193: a hard delegation failure (e.g. run_code's coding model + // 404ing) must be surfaced so the orchestrator cannot narrate a + // plausible success. The envelope states the task did not run, tells + // the model not to fabricate output, and preserves the root error. + let msg = format_subagent_failure( + "run_code", + "openhuman API error (404): model 'davinci-002' does not support \ + the chat-completions API", + ); + assert!(msg.contains("run_code failed"), "names the tool: {msg}"); + assert!( + msg.contains("did not complete"), + "states no completion: {msg}" + ); + assert!( + msg.to_lowercase().contains("do not treat this as success") + && msg.contains("fabricate"), + "warns against fabricated success: {msg}" + ); + assert!( + msg.contains("davinci-002") && msg.contains("404"), + "preserves the root error: {msg}" + ); + } } diff --git a/src/openhuman/inference/provider/compatible.rs b/src/openhuman/inference/provider/compatible.rs index a851f59e9..a82fa972f 100644 --- a/src/openhuman/inference/provider/compatible.rs +++ b/src/openhuman/inference/provider/compatible.rs @@ -137,6 +137,43 @@ impl OpenAiCompatibleProvider { } } + /// Build an actionable error for a completion-only model that was routed + /// to `/v1/chat/completions`. OpenHuman only speaks the chat-completions + /// API (with an optional `/v1/responses` fallback) — a completion-only / + /// base model 404s here and the responses fallback cannot rescue it, so we + /// surface the model name and concrete remediation instead of an opaque + /// "responses fallback failed" chain. See issue #3193. + fn completion_only_model_message(&self, model: &str, sanitized: &str) -> String { + format!( + "{name} API error (404): model '{model}' does not support the \ + chat-completions API that OpenHuman uses — it appears to be a \ + completion-only / base model. Assign a chat-capable model to this \ + provider (e.g. in Settings → AI), or pick a different model. \ + Provider detail: {sanitized}", + name = self.name, + ) + } + + /// Guard shared by every chat-completions 404 handler: if the body shows a + /// completion-only model, return the actionable error so the caller can + /// fail fast instead of attempting the futile `/v1/responses` fallback. + /// `None` means "not this case — proceed with normal fallback/enrich". + /// See issue #3193. + fn completion_only_404_guard( + &self, + status: reqwest::StatusCode, + sanitized: &str, + model: &str, + ) -> Option { + if Self::is_completion_only_model_404(status, sanitized) { + Some(anyhow::anyhow!( + self.completion_only_model_message(model, sanitized) + )) + } else { + None + } + } + /// Create a provider with a custom User-Agent header. /// /// Some providers (for example Kimi Code) require a specific User-Agent @@ -818,6 +855,24 @@ impl OpenAiCompatibleProvider { Self::is_native_tool_schema_unsupported(reqwest::StatusCode::BAD_REQUEST, error) } + /// Detect a 404 whose body says the model is completion-only and cannot be + /// served from `/v1/chat/completions` (OpenAI: "This is not a chat model + /// and thus not supported in the v1/chat/completions endpoint. Did you + /// mean to use v1/completions?"). When this fires, attempting the + /// `/v1/responses` fallback is futile, so callers should fail fast with an + /// actionable message via [`completion_only_model_message`]. The match is + /// deliberately tight so ordinary "model does not exist" 404s are NOT + /// caught (those should keep their existing fallback / enrich behaviour). + /// See issue #3193. + fn is_completion_only_model_404(status: reqwest::StatusCode, error: &str) -> bool { + if status != reqwest::StatusCode::NOT_FOUND { + return false; + } + let lower = error.to_lowercase(); + lower.contains("not a chat model") + || (lower.contains("v1/chat/completions") && lower.contains("v1/completions")) + } + /// Streaming variant of the native-tools chat path. /// /// Sends the request with `stream: true`, consumes the upstream SSE @@ -1363,6 +1418,12 @@ impl Provider for OpenAiCompatibleProvider { let error = response.text().await?; let sanitized = super::sanitize_api_error(&error); + // A completion-only model 404s here and the /v1/responses fallback + // cannot rescue it — fail fast with actionable guidance (#3193). + if let Some(err) = self.completion_only_404_guard(status, &sanitized, model) { + return Err(err); + } + if status == reqwest::StatusCode::NOT_FOUND && self.supports_responses_fallback { return self .chat_via_responses(credential, &fallback_messages, model) @@ -1530,18 +1591,38 @@ impl Provider for OpenAiCompatibleProvider { if !response.status().is_success() { let status = response.status(); - // Mirror chat_with_system: 404 may mean this provider uses the Responses API - if status == reqwest::StatusCode::NOT_FOUND && self.supports_responses_fallback { - return self - .chat_via_responses(credential, &effective_messages, model) - .await - .map_err(|responses_err| { - let fb = super::format_anyhow_chain(&responses_err); - anyhow::anyhow!( - "{} API error (chat completions unavailable; responses fallback failed: {fb})", - self.name - ) - }); + // A 404 may mean this provider uses the Responses API, OR that the + // model is completion-only. Read the body once so we can tell the + // two apart (#3193) — only the 404 branch needs it; the response is + // not used again here, so `api_error` below still owns the rest. + if status == reqwest::StatusCode::NOT_FOUND { + let error = response.text().await?; + let sanitized = super::sanitize_api_error(&error); + + // Completion-only model: the responses fallback can't help — + // fail fast with actionable guidance. + if let Some(err) = self.completion_only_404_guard(status, &sanitized, model) { + return Err(err); + } + + if self.supports_responses_fallback { + return self + .chat_via_responses(credential, &effective_messages, model) + .await + .map_err(|responses_err| { + let fb = super::format_anyhow_chain(&responses_err); + anyhow::anyhow!( + "{} API error ({status}): {sanitized} (chat completions unavailable; responses fallback failed: {fb})", + self.name + ) + }); + } + + let enriched = self.enrich_404_message( + format!("{} API error ({status}): {sanitized}", self.name), + status, + ); + return Err(anyhow::anyhow!("{enriched}")); } let err = super::api_error(&self.name, response).await; @@ -1879,6 +1960,12 @@ impl Provider for OpenAiCompatibleProvider { }); } + // A completion-only model 404s here and the /v1/responses fallback + // cannot rescue it — fail fast with actionable guidance (#3193). + if let Some(err) = self.completion_only_404_guard(status, &sanitized, model) { + return Err(err); + } + if status == reqwest::StatusCode::NOT_FOUND && self.supports_responses_fallback { return self .chat_via_responses(credential, &effective_messages, model) diff --git a/src/openhuman/inference/provider/compatible_tests.rs b/src/openhuman/inference/provider/compatible_tests.rs index beb737c97..a02976a93 100644 --- a/src/openhuman/inference/provider/compatible_tests.rs +++ b/src/openhuman/inference/provider/compatible_tests.rs @@ -2181,3 +2181,123 @@ fn convert_tool_specs_dedups_many_duplicates() { .collect(); assert_eq!(names, vec!["x", "y", "z"]); } + +// ── #3193: completion-only model 404 detection + actionable message ────────── + +#[test] +fn completion_only_model_404_detected_from_openai_signature() { + // The exact body OpenAI returns when a completion-only/base model is sent + // to /v1/chat/completions. + let body = "This is not a chat model and thus not supported in the \ + v1/chat/completions endpoint. Did you mean to use v1/completions?"; + assert!(OpenAiCompatibleProvider::is_completion_only_model_404( + reqwest::StatusCode::NOT_FOUND, + body + )); +} + +#[test] +fn completion_only_model_404_ignores_ordinary_not_found() { + // A "model does not exist" 404 must NOT be misclassified — it should keep + // its existing fallback / enrich behaviour, not get the completion-only + // message. + let body = "The model `gpt-9o` does not exist or you do not have access to it."; + assert!(!OpenAiCompatibleProvider::is_completion_only_model_404( + reqwest::StatusCode::NOT_FOUND, + body + )); +} + +#[test] +fn completion_only_model_404_requires_404_status() { + // Same phrasing under a non-404 status is not the completion-only case. + let body = "not a chat model"; + assert!(!OpenAiCompatibleProvider::is_completion_only_model_404( + reqwest::StatusCode::BAD_REQUEST, + body + )); +} + +#[test] +fn completion_only_message_names_model_and_remediation() { + let p = make_provider("openhuman", "https://api.example.com/v1", Some("k")); + let msg = p.completion_only_model_message( + "davinci-002", + "This is not a chat model ... Did you mean to use v1/completions?", + ); + assert!( + msg.contains("davinci-002"), + "names the offending model: {msg}" + ); + assert!( + msg.contains("completion-only") && msg.contains("chat-completions"), + "explains the capability mismatch: {msg}" + ); + assert!( + msg.contains("chat-capable model"), + "states the remediation: {msg}" + ); +} + +#[test] +fn completion_only_404_guard_fires_only_on_signature() { + let p = make_provider("openhuman", "https://api.example.com/v1", Some("k")); + // Matches → Some(actionable error). + let hit = p.completion_only_404_guard( + reqwest::StatusCode::NOT_FOUND, + "This is not a chat model. Did you mean to use v1/completions?", + "davinci-002", + ); + let err = hit.expect("guard should fire on the completion-only signature"); + assert!(err.to_string().contains("davinci-002")); + // Ordinary not-found → None (normal fallback/enrich path is preserved). + assert!(p + .completion_only_404_guard( + reqwest::StatusCode::NOT_FOUND, + "The model `gpt-9o` does not exist.", + "gpt-9o" + ) + .is_none()); +} + +#[tokio::test] +async fn completion_only_404_fails_fast_without_responses_fallback() { + // End-to-end over the wire: a completion-only 404 must short-circuit with + // the actionable message and NOT attempt /v1/responses (not mounted here — + // if the guard regressed, the error would instead read "responses fallback + // failed"). Provider has the fallback ENABLED (default `new`), proving the + // guard pre-empts it. #3193. + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v1/chat/completions")) + .respond_with(ResponseTemplate::new(404).set_body_json(serde_json::json!({ + "error": { + "message": "This is not a chat model and thus not supported in the \ + v1/chat/completions endpoint. Did you mean to use v1/completions?", + "type": "invalid_request_error" + } + }))) + .mount(&server) + .await; + + let provider = OpenAiCompatibleProvider::new( + "openhuman", + &format!("{}/v1", server.uri()), + Some("key"), + AuthStyle::Bearer, + ); + + let err = provider + .chat_with_history(&[ChatMessage::user("write a file")], "davinci-002", 0.0) + .await + .expect_err("completion-only model must error"); + let msg = err.to_string(); + assert!( + msg.contains("davinci-002") && msg.contains("chat-capable model"), + "expected actionable completion-only message, got: {msg}" + ); + assert!( + !msg.contains("responses fallback failed"), + "guard must pre-empt the responses fallback, got: {msg}" + ); +}