Enhance request handling and response structure by mjavadalavi · Pull Request #5 · Securiteru/codex-openai-proxy

mjavadalavi · 2026-02-02T10:13:40Z

Updated reqwest dependency to include additional features for improved HTTP requests.
Modified ChatCompletionsRequest and ChatMessage structs to support new fields for tool calls and reasoning.
Improved content handling in convert_chat_to_responses to accommodate tool call messages.
Enhanced error handling in proxy_request to provide clearer backend error messages.
Refactored response building to support streaming and structured output for tool calls.

- Updated `reqwest` dependency to include additional features for improved HTTP requests. - Modified `ChatCompletionsRequest` and `ChatMessage` structs to support new fields for tool calls and reasoning. - Improved content handling in `convert_chat_to_responses` to accommodate tool call messages. - Enhanced error handling in `proxy_request` to provide clearer backend error messages. - Refactored response building to support streaming and structured output for tool calls.

Copilot

Pull request overview

This PR significantly enhances the proxy server to better support tool calls, improve request handling, and add OpenAI API integration. The changes transform the proxy from a simple placeholder to a more functional system capable of handling complex chat interactions with tool calls and reasoning.

Changes:

Added support for tool calls in chat messages, including parsing, normalization, and response serialization
Integrated OpenAI API as an alternative backend alongside ChatGPT, with automatic selection based on configuration
Enhanced error handling to provide backend error details instead of generic fallback responses

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 16 comments.

File	Description
Cargo.toml	Updated reqwest dependency to use rustls-tls with compression support features
src/main.rs	Major refactoring including tool call support, dual backend routing, enhanced message parsing, SSE response building, and comprehensive response parsing logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-02T10:18:49Z

+                    }
+                }
+            }
+            Value::String("auto".to_string())


The normalize_tool_choice function doesn't handle all possible tool_choice formats correctly. When a Value::Object is provided but doesn't match the expected patterns, it falls back to "auto", which may not preserve the original intent. Consider returning an error or the original value unchanged if it's a valid object that doesn't match expected patterns.

Suggested change

Value::String("auto".to_string())

// Preserve the original object if it doesn't match expected patterns

Value::Object(obj)

Copilot · 2026-02-02T10:18:50Z

+fn content_item_for_role(role: &str, text: String) -> ContentItem {
+    let role = role.trim();
+    if role.eq_ignore_ascii_case("assistant") || role.eq_ignore_ascii_case("tool") {
+        ContentItem::OutputText { text }
+    } else {
+        ContentItem::InputText { text }
+    }


The function maps "tool" role to OutputText, but in line 417 of normalize_role, "tool" is normalized to "user". This creates an inconsistency where tool messages are treated as user-role but with OutputText content type. Consider aligning these two functions so that the role normalization and content type selection are consistent.

Copilot · 2026-02-02T10:18:50Z

+            if msg.role.trim().eq_ignore_ascii_case("tool") {
+                if let Some(tool_call_id) = msg.tool_call_id.as_deref() {
+                    if content_text.is_empty() {
+                        content_text = format!("Tool result for {}.", tool_call_id);
+                    } else {
+                        content_text = format!("Tool result for {}: {}", tool_call_id, content_text);
+                    }
+                }
+            }


This code checks msg.role directly instead of using normalized_role, which could lead to inconsistent behavior. If the role has extra whitespace or different casing, the normalization on line 200 would handle it, but this check on line 207 uses the original role. Consider using normalized_role for consistency, or apply the same trim and case-insensitive logic here as well.

Copilot · 2026-02-02T10:18:50Z

+        .or_else(|| item.get("function").and_then(|f| f.get("arguments")));
+    let arguments = match args_value {
+        Some(Value::String(s)) => s.clone(),
+        Some(Value::Object(_)) | Some(Value::Array(_)) => args_value.unwrap().to_string(),


The code uses unwrap() on args_value at line 833, which will panic if args_value is None. This branch is only reachable when args_value is Some(Value::Object()) or Some(Value::Array()), so the unwrap is safe. However, for better code clarity and to avoid potential future issues if the logic changes, consider restructuring to avoid unwrap, perhaps by matching on the Some variants directly.

Suggested change

Some(Value::Object(_)) | Some(Value::Array(_)) => args_value.unwrap().to_string(),

Some(v @ Value::Object(_)) | Some(v @ Value::Array(_)) => v.to_string(),

Copilot · 2026-02-02T10:18:51Z

-
-mod improved_response;
+use std::collections::HashMap;



The module declaration for 'improved_response' has been removed (line 8 in the old code), but the file 'src/improved_response.rs' still exists in the repository. This orphaned file should be deleted to keep the codebase clean, or the module should be re-added if it's still needed.

Suggested change

mod improved_response;

Copilot · 2026-02-02T10:18:53Z

+fn normalize_tools(tools: Vec<Value>) -> Vec<Value> {
+    let mut normalized = Vec::new();
+
+    for tool in tools {
+        let Value::Object(obj) = tool else {
+            continue;
+        };
+
+        let tool_type = obj.get("type").and_then(|v| v.as_str());
+        if tool_type != Some("function") {
+            normalized.push(Value::Object(obj));
+            continue;
+        }
+
+        if obj.get("name").and_then(|v| v.as_str()).is_some() {
+            normalized.push(Value::Object(obj));
+            continue;
+        }
+
+        if let Some(Value::Object(function)) = obj.get("function") {
+            let name = function.get("name").and_then(|v| v.as_str());
+            if let Some(name) = name {
+                let mut normalized_tool = serde_json::Map::new();
+                normalized_tool.insert("type".to_string(), Value::String("function".to_string()));
+                normalized_tool.insert("name".to_string(), Value::String(name.to_string()));
+                if let Some(description) = function.get("description") {
+                    normalized_tool.insert("description".to_string(), description.clone());
+                }
+                if let Some(parameters) = function.get("parameters") {
+                    normalized_tool.insert("parameters".to_string(), parameters.clone());
+                }
+                if let Some(strict) = function.get("strict") {
+                    normalized_tool.insert("strict".to_string(), strict.clone());
+                }
+                normalized.push(Value::Object(normalized_tool));
+            }
+        }
+    }
+
+    normalized
+}


The normalize_tools function silently drops tools that don't match expected patterns (non-objects, or function tools without a name). This could lead to unexpected behavior where tools are silently ignored. Consider logging a warning or returning an error when tools can't be normalized, so users are aware of the issue.

Copilot · 2026-02-02T10:18:53Z

+    #[allow(dead_code)]
    temperature: Option<f32>,
+    #[allow(dead_code)]


The temperature and max_tokens fields are marked with allow(dead_code), but they're actually used in destructuring on lines 187-188. These fields should either be used in the request (passed to the backend) or removed from the struct if they're intentionally ignored. The dead_code attribute is misleading here.

Suggested change

#[allow(dead_code)]

temperature: Option<f32>,

#[allow(dead_code)]

temperature: Option<f32>,

Copilot · 2026-02-02T10:18:53Z

-                completion_tokens: 0,
-                total_tokens: 0,
-            }),
+            usage: None,


The usage field is now set to None instead of providing token counts. While this may be acceptable for some use cases, many API consumers expect usage information for tracking costs and performance. Consider parsing usage data from the backend response if available, or documenting why usage tracking is not provided.

Copilot · 2026-02-02T10:18:54Z

 chrono = { version = "0.4", features = ["serde"] }
 env_logger = "0.10"
-reqwest = { version = "0.11", features = ["json"] }
+reqwest = { version = "0.11", default-features = false, features = ["json", "rustls-tls", "gzip", "brotli", "deflate"] }


The reqwest dependency now disables default features and explicitly enables rustls-tls instead of the default native-tls. While this is often preferred for cross-compilation, it may cause issues on systems where rustls doesn't work well or where native certificates are required. Consider documenting this change and ensuring it's intentional, or provide a feature flag to allow users to choose between rustls and native-tls.

Copilot · 2026-02-02T10:18:54Z

+        if !status.is_success() {
+            bail!(
+                "ChatGPT backend error ({}): {}",
+                status,
+                response_text


The error handling exposes the full backend error response text to the client (line 320). This could leak sensitive information such as internal error details, stack traces, or configuration information. Consider sanitizing or filtering the error message before returning it to the client, especially in production environments.

Suggested change

if !status.is_success() {

bail!(

"ChatGPT backend error ({}): {}",

status,

response_text

let safe_response_text = if cfg!(debug_assertions) {

// In debug builds, include full backend response for easier debugging.

response_text.clone()

} else {

// In non-debug builds, avoid exposing full backend error details to clients.

"[redacted backend error body]".to_string()

};

if !status.is_success() {

bail!(

"ChatGPT backend error ({}): {}",

status,

safe_response_text

Copilot AI review requested due to automatic review settings February 2, 2026 10:13

Copilot started reviewing on behalf of mjavadalavi February 2, 2026 10:13 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance request handling and response structure#5

Enhance request handling and response structure#5
mjavadalavi wants to merge 1 commit intoSecuriteru:mainfrom
mjavadalavi:main

mjavadalavi commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Value::String("auto".to_string())
	// Preserve the original object if it doesn't match expected patterns
	Value::Object(obj)

	Some(Value::Object(_)) \| Some(Value::Array(_)) => args_value.unwrap().to_string(),
	Some(v @ Value::Object(_)) \| Some(v @ Value::Array(_)) => v.to_string(),

Conversation

mjavadalavi commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants