From 77511bbf5b4bd17b037a82ce0b56b5db185e0782 Mon Sep 17 00:00:00 2001 From: serrrfirat Date: Fri, 3 Apr 2026 16:46:59 +0300 Subject: [PATCH] fix(llm): add sanitize_tool_messages to OpenAiCodexProvider The OpenAI Codex provider was missing the sanitize_tool_messages() call that all other providers use to rewrite orphaned tool results as user messages. This caused HTTP 400 errors when conversation history contained tool result messages whose corresponding tool calls were dropped during context compaction or thread resume. Adds the call to both complete() and complete_with_tools(), matching the pattern used by rig_adapter, nearai_chat, and bedrock providers. Fixes #1969 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/llm/openai_codex_provider.rs | 64 +++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/src/llm/openai_codex_provider.rs b/src/llm/openai_codex_provider.rs index f1f688ac8ad..d4b2118ff33 100644 --- a/src/llm/openai_codex_provider.rs +++ b/src/llm/openai_codex_provider.rs @@ -259,7 +259,9 @@ impl LlmProvider for OpenAiCodexProvider { } async fn complete(&self, request: CompletionRequest) -> Result { - let body = self.build_request_body(&request.messages, None); + let mut messages = request.messages; + crate::llm::provider::sanitize_tool_messages(&mut messages); + let body = self.build_request_body(&messages, None); let parsed = self.send_request(body).await?; Ok(CompletionResponse { @@ -276,6 +278,9 @@ impl LlmProvider for OpenAiCodexProvider { &self, request: ToolCompletionRequest, ) -> Result { + let mut messages = request.messages; + crate::llm::provider::sanitize_tool_messages(&mut messages); + // Build a reverse map so we can translate sanitized names back to originals. // Only needed when sanitization actually changes a name (e.g. MCP tools with dots). let name_map: std::collections::HashMap = request @@ -291,7 +296,7 @@ impl LlmProvider for OpenAiCodexProvider { }) .collect(); - let body = self.build_request_body(&request.messages, Some(&request.tools)); + let body = self.build_request_body(&messages, Some(&request.tools)); let mut parsed = self.send_request(body).await?; // Reverse-map sanitized tool names back to originals so the caller @@ -1223,4 +1228,59 @@ data: {"type":"response.completed","response":{"status":"completed","usage":{"in } assert_eq!(tc.name, "mcp.server.search"); } + + /// Regression test for #1969: orphaned tool results must be sanitized + /// before building the request body, otherwise the Responses API returns + /// HTTP 400 because function_call_output references a non-existent call_id. + #[test] + fn test_build_request_sanitizes_orphaned_tool_results() { + use crate::llm::provider::sanitize_tool_messages; + + // An orphaned tool result: no preceding assistant message with a + // matching tool_call for "call_orphan". + let mut messages = vec![ + ChatMessage::system("You are helpful"), + ChatMessage::user("hello"), + ChatMessage::assistant("I'll use a tool"), + ChatMessage::tool_result("call_orphan", "search", "found 3 results"), + ]; + + // Before sanitization the message is Role::Tool with a tool_call_id. + assert_eq!(messages[3].role, Role::Tool); + assert_eq!(messages[3].tool_call_id, Some("call_orphan".to_string())); + + sanitize_tool_messages(&mut messages); + + // After sanitization it must be rewritten to a user message. + assert_eq!(messages[3].role, Role::User); + assert!(messages[3].content.contains("[Tool `search` returned:")); + assert!(messages[3].content.contains("found 3 results")); + assert!(messages[3].tool_call_id.is_none()); + assert!(messages[3].name.is_none()); + + // Verify the rewritten message converts to a user input item (not + // a function_call_output that would cause HTTP 400). + let jwt = make_test_jwt("acct_test"); + let provider = OpenAiCodexProvider::new( + "gpt-5.3-codex", + "https://chatgpt.com/backend-api/codex", + &jwt, + 300, + ) + .unwrap(); + + let body = provider.build_request_body(&messages, None); + let input = body["input"].as_array().unwrap(); + + // Should have 3 non-system items: user, assistant, rewritten-user + assert_eq!(input.len(), 3); + // The last item must be a user message, not a function_call_output + assert_eq!(input[2]["role"], "user"); + assert!( + input[2]["content"][0]["text"] + .as_str() + .unwrap() + .contains("[Tool `search` returned:") + ); + } }