feat(model_gateway): support custom tool in OpenAI responses API#1000
feat(model_gateway): support custom tool in OpenAI responses API#1000BenDing96 wants to merge 3 commits intolightseekorg:mainfrom
Conversation
BenDing96
requested review from
CatherineSue,
key4ng and
slin1237
as code owners
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
github-actions
bot
added
grpc
📝 WalkthroughWalkthroughIntroduces custom tool call support across the codebase by adding Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @BenDing96, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
|
Hi @BenDing96, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch: git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease |
mergify
bot
added
the
needs-rebase
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements support for "custom tools" throughout the system, adding new types like CustomTool and CustomToolCall to the protocol and updating the gateway to handle these variants. Key changes include specialized validation, streaming event emission for custom tool inputs, and logic to map custom tools to the underlying chat completion format. The PR also updates persistence and history loading to support the new item types. A review comment identifies an opportunity to reduce code duplication by extracting the logic for collecting custom tool names into a helper method on ResponsesRequest.
| let custom_tool_names: std::collections::HashSet<&str> = responses_request | ||
| .tools | ||
| .as_ref() | ||
| .map(|tools| { | ||
| tools | ||
| .iter() | ||
| .filter_map(|t| match t { | ||
| ResponseTool::Custom(ct) => Some(ct.name.as_str()), | ||
| _ => None, | ||
| }) | ||
| .collect() | ||
| }) | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
This logic to collect custom tool names into a HashSet is duplicated in several places in this PR. To improve code reuse and maintainability, consider adding a helper method to ResponsesRequest.
For example, you could add this to crates/protocols/src/responses.rs:
impl ResponsesRequest {
pub fn custom_tool_names(&self) -> std::collections::HashSet<&str> {
self.tools
.as_deref()
.unwrap_or(&[])
.iter()
.filter_map(|t| {
if let ResponseTool::Custom(ct) = t {
Some(ct.name.as_str())
} else {
None
}
})
.collect()
}
}Then you can simplify this block to:
let custom_tool_names = responses_request.custom_tool_names();This would also simplify similar blocks in model_gateway/src/routers/grpc/regular/responses/conversions.rs and model_gateway/src/routers/grpc/regular/responses/streaming.rs.
References
- Extract duplicated logic into a shared helper function to improve maintainability and reduce redundancy.
- To prevent vulnerabilities from duplicate entries, use data structures that inherently enforce uniqueness, such as HashSet, instead of manually deduplicating collections like Vec.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/data_connector/src/core.rs`:
- Line 296: Update the ID-prefix mapping in crates/data_connector/src/core.rs so
it matches the router-side mapping in
model_gateway/src/routers/persistence_utils.rs: ensure both "custom_tool_call"
and "custom_tool_call_output" map to "ctc" (instead of only one mapping or
divergent prefixes) and remove/merge any duplicate hand-written maps around the
mapping block (see the entries near lines 725-738 and 767) so the two keys
consistently use "ctc".
In `@crates/protocols/src/responses.rs`:
- Around line 1238-1245: Currently only empty custom.names are rejected in
ResponseTool::Custom; add validation to reject duplicate tool names across
function and custom tool variants. When validating tools in the responses
parsing/validation path (e.g., where ResponseTool::Custom and
ResponseTool::Function are handled), collect the set of names from
ResponseTool::Function and then, when encountering a ResponseTool::Custom with a
name that already exists in that set (and vice‑versa if functions are validated
after customs), return a ValidationError (use ValidationError::new with an
appropriate code like "duplicate_tool_name") whose message clearly states that
the tool name is already used (e.g., "Duplicate tool name: 'foo' used by both
function and custom tool"). Ensure you reference and check the custom.name and
the corresponding function tool name fields so conflicting names are rejected.
In `@model_gateway/src/routers/grpc/common/responses/utils.rs`:
- Around line 128-136: The conversion for ResponseTool::Custom(ct) currently
discards ct.format and sets Function.parameters to an empty object; update the
ResponseTool::Custom(ct) branch so that Function.parameters is built from
ct.format (convert ct.format into the appropriate JSON Schema/serde_json::Value)
and propagate any related flags (e.g., strict) so the Tool/Function preserves
grammar-constrained input; locate the ResponseTool::Custom(ct) match arm and
replace the hardcoded serde_json::json!({}) with a serialization of ct.format
(or a constructed schema from ct.format) and ensure ct.format is not ignored
when constructing Tool and Function.
In `@model_gateway/src/routers/grpc/harmony/builder.rs`:
- Around line 120-124: ResponseTool::Custom mapping currently drops the tool
format by calling ToolDescription::new(..., None); change it to forward the
custom tool's format (ct.format) into ToolDescription::new so formatted custom
tools retain their constraints when the Harmony prompt is built—i.e., replace
the None with ct.format.clone() or ct.format (matching the type) in the
ResponseTool::Custom arm.
In `@model_gateway/src/routers/grpc/regular/responses/conversions.rs`:
- Around line 136-172: The assistant tool-call replay uses the wrapper
conversation item id (`id`) instead of the backend tool identifier (`call_id`),
causing mismatched tool_call.id vs tool message tool_call_id; update the
ResponseInputOutputItem::CustomToolCall branch to use the actual backend call id
(call_id) for the ToolCall.id and ensure the Tool message uses the same call_id
(and likewise ensure ResponseInputOutputItem::CustomToolCallOutput uses call_id
for tool_call_id), i.e., replace usages of id.clone() with the backend call
identifier so ToolCall { id: ... } and ChatMessage::Tool { tool_call_id: ... }
consistently use call_id.
In `@model_gateway/src/routers/grpc/regular/responses/streaming.rs`:
- Around line 382-419: The conversion to CustomToolCall is producing empty
call_id because streamed deltas only set the streamed tool-call id in the
FunctionToolCall.id field (process_chunk() fills id, name, arguments but leaves
call_id empty); update the conversion loop that iterates self.tool_calls (in
streaming.rs) so that when matching ResponseOutputItem::FunctionToolCall you
populate call_id from the FunctionToolCall.id (or from the streamed tool-call id
field) before constructing ResponseOutputItem::CustomToolCall; ensure you copy
id -> call_id (or use a non-empty call identifier derived from id) so
CustomToolCall.call_id is populated for downstream consumers.
In `@model_gateway/src/routers/openai/responses/utils.rs`:
- Line 234: The match arm for ResponseTool::Custom currently returns None which
causes custom tools to be dropped when restoring final response tools; update
the ResponseTool handling so that ResponseTool::Custom(_) returns Some(...)
preserving the custom tool entry (i.e., return the custom tool value instead of
None) so that restoring MCP/builtin tools merges with rather than overwrites
response.tools; locate the match that contains ResponseTool::Custom and change
its branch to return the appropriate Some variant that contains the custom tool.
In `@model_gateway/tests/spec/responses.rs`:
- Around line 1521-1537: The test test_validate_custom_tool_call_output_empty is
currently passing due to ResponsesRequest::validate rejecting Items when there
is no Message item, so update the fixture to ensure the empty-output validator
is actually exercised: construct a ResponsesRequest whose input contains both
the ResponseInputOutputItem::CustomToolCallOutput with output set to empty AND a
valid message item (e.g., a ResponseInput::Message entry) so validate() advances
to the CustomToolCallOutput check, then assert the specific validation error for
CustomToolCallOutput.output (or an expected error code/message) instead of just
is_err(); reference ResponsesRequest::validate, ResponseInput::Items, and
ResponseInputOutputItem::CustomToolCallOutput.output when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e69b0c6e-151d-4ae6-bb61-5e0299743b03
📒 Files selected for processing (14)
crates/data_connector/src/core.rscrates/protocols/src/event_types.rscrates/protocols/src/responses.rsmodel_gateway/src/routers/grpc/common/responses/streaming.rsmodel_gateway/src/routers/grpc/common/responses/utils.rsmodel_gateway/src/routers/grpc/harmony/builder.rsmodel_gateway/src/routers/grpc/harmony/responses/non_streaming.rsmodel_gateway/src/routers/grpc/regular/responses/conversions.rsmodel_gateway/src/routers/grpc/regular/responses/streaming.rsmodel_gateway/src/routers/openai/responses/history.rsmodel_gateway/src/routers/openai/responses/streaming.rsmodel_gateway/src/routers/openai/responses/utils.rsmodel_gateway/src/routers/persistence_utils.rsmodel_gateway/tests/spec/responses.rs
| "mcp_call" => "mcp", | ||
| "mcp_list_tools" => "mcpl", | ||
| "function_call" => "fc", | ||
| "custom_tool_call" => "ctc", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Keep custom-tool ID prefixes aligned with the router-side mapping.
The new branch/tests cover custom_tool_call, but model_gateway/src/routers/persistence_utils.rs already maps both custom_tool_call and custom_tool_call_output to ctc. Keeping separate hand-written maps here makes the new custom-tool IDs easy to drift again.
💡 Minimal alignment diff
- "custom_tool_call" => "ctc",
+ "custom_tool_call" | "custom_tool_call_output" => "ctc",
@@
let test_cases = vec![
("message", "msg_"),
("reasoning", "rs_"),
("mcp_call", "mcp_"),
("mcp_list_tools", "mcpl_"),
("function_call", "fc_"),
("custom_tool_call", "ctc_"),
+ ("custom_tool_call_output", "ctc_"),
];Also applies to: 725-738, 767-767
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/data_connector/src/core.rs` at line 296, Update the ID-prefix mapping
in crates/data_connector/src/core.rs so it matches the router-side mapping in
model_gateway/src/routers/persistence_utils.rs: ensure both "custom_tool_call"
and "custom_tool_call_output" map to "ctc" (instead of only one mapping or
divergent prefixes) and remove/merge any duplicate hand-written maps around the
mapping block (see the entries near lines 725-738 and 767) so the two keys
consistently use "ctc".
| if let ResponseTool::Custom(custom) = tool { | ||
| if custom.name.is_empty() { | ||
| let mut e = ValidationError::new("missing_required_parameter"); | ||
| e.message = | ||
| Some(format!("Missing required parameter: 'tools[{idx}].name'.").into()); | ||
| return Err(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Reject duplicate names across function and custom tools.
model_gateway/src/routers/grpc/harmony/responses/non_streaming.rs now decides whether to emit FunctionToolCall or CustomToolCall by checking the tool name against a custom-name set. With only the validation here, a request can still declare both a function tool and a custom tool named foo, which makes the output type ambiguous and currently serializes any foo call as custom_tool_call.
💡 Suggested fix
fn validate_response_tools(tools: &[ResponseTool]) -> Result<(), ValidationError> {
// MCP server_label must be present and unique (case-insensitive).
let mut seen_mcp_labels: HashSet<String> = HashSet::new();
+ let mut seen_named_tools: HashSet<String> = HashSet::new();
for (idx, tool) in tools.iter().enumerate() {
+ if let Some(name) = match tool {
+ ResponseTool::Function(ft) => Some(ft.function.name.as_str()),
+ ResponseTool::Custom(custom) => Some(custom.name.as_str()),
+ _ => None,
+ } {
+ if !seen_named_tools.insert(name.to_string()) {
+ let mut e = ValidationError::new("duplicate_tool_name");
+ e.message = Some(
+ format!("Duplicate tool name '{name}' found in 'tools' parameter.").into(),
+ );
+ return Err(e);
+ }
+ }
+
if let ResponseTool::Custom(custom) = tool {
if custom.name.is_empty() {
let mut e = ValidationError::new("missing_required_parameter");
e.message =
Some(format!("Missing required parameter: 'tools[{idx}].name'.").into());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/protocols/src/responses.rs` around lines 1238 - 1245, Currently only
empty custom.names are rejected in ResponseTool::Custom; add validation to
reject duplicate tool names across function and custom tool variants. When
validating tools in the responses parsing/validation path (e.g., where
ResponseTool::Custom and ResponseTool::Function are handled), collect the set of
names from ResponseTool::Function and then, when encountering a
ResponseTool::Custom with a name that already exists in that set (and vice‑versa
if functions are validated after customs), return a ValidationError (use
ValidationError::new with an appropriate code like "duplicate_tool_name") whose
message clearly states that the tool name is already used (e.g., "Duplicate tool
name: 'foo' used by both function and custom tool"). Ensure you reference and
check the custom.name and the corresponding function tool name fields so
conflicting names are rejected.
| ResponseTool::Custom(ct) => Some(Tool { | ||
| tool_type: "function".to_string(), | ||
| function: Function { | ||
| name: ct.name.clone(), | ||
| description: ct.description.clone(), | ||
| parameters: serde_json::json!({}), | ||
| strict: None, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
tools[].format becomes a no-op on the self-hosted gRPC conversion path.
This conversion turns every custom tool into an unconstrained parameterless function and drops ct.format entirely. Requests that depend on grammar-constrained custom input will still be accepted, but the backend never receives the constraint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model_gateway/src/routers/grpc/common/responses/utils.rs` around lines 128 -
136, The conversion for ResponseTool::Custom(ct) currently discards ct.format
and sets Function.parameters to an empty object; update the
ResponseTool::Custom(ct) branch so that Function.parameters is built from
ct.format (convert ct.format into the appropriate JSON Schema/serde_json::Value)
and propagate any related flags (e.g., strict) so the Tool/Function preserves
grammar-constrained input; locate the ResponseTool::Custom(ct) match arm and
replace the hardcoded serde_json::json!({}) with a serialization of ct.format
(or a constructed schema from ct.format) and ensure ct.format is not ignored
when constructing Tool and Function.
| ResponseTool::Custom(ct) => Some(ToolDescription::new( | ||
| ct.name.clone(), | ||
| ct.description.clone().unwrap_or_default(), | ||
| None, | ||
| )), |
There was a problem hiding this comment.
Harmony request building also discards custom-tool format constraints.
ToolDescription::new(..., None) strips ct.format before the Harmony prompt is built, so formatted custom tools are treated as unconstrained free-form calls on this path too.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model_gateway/src/routers/grpc/harmony/builder.rs` around lines 120 - 124,
ResponseTool::Custom mapping currently drops the tool format by calling
ToolDescription::new(..., None); change it to forward the custom tool's format
(ct.format) into ToolDescription::new so formatted custom tools retain their
constraints when the Harmony prompt is built—i.e., replace the None with
ct.format.clone() or ct.format (matching the type) in the ResponseTool::Custom
arm.
| ResponseInputOutputItem::CustomToolCall { | ||
| id, | ||
| name, | ||
| input, | ||
| output, | ||
| .. | ||
| } => { | ||
| // Custom tool call from history - convert to function tool call format | ||
| messages.push(ChatMessage::Assistant { | ||
| content: None, | ||
| name: None, | ||
| tool_calls: Some(vec![ToolCall { | ||
| id: id.clone(), | ||
| tool_type: "function".to_string(), | ||
| function: FunctionCallResponse { | ||
| name: name.clone(), | ||
| arguments: Some(input.clone()), | ||
| }, | ||
| }]), | ||
| reasoning_content: None, | ||
| }); | ||
|
|
||
| if let Some(output_text) = output { | ||
| messages.push(ChatMessage::Tool { | ||
| content: MessageContent::Text(output_text.clone()), | ||
| tool_call_id: id.clone(), | ||
| }); | ||
| } | ||
| } | ||
| ResponseInputOutputItem::CustomToolCallOutput { | ||
| call_id, output, .. | ||
| } => { | ||
| messages.push(ChatMessage::Tool { | ||
| content: MessageContent::Text(output.clone()), | ||
| tool_call_id: call_id.clone(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Use call_id, not the conversation item id, when replaying custom tool history.
Line 148 and Line 161 currently reuse the item id. For streamed custom tools, id is the wrapper conversation item id (ctc_*), while the actual backend tool-call identifier lives in call_id. Replaying persisted history through chat will then produce an assistant tool_call.id that does not match the later tool message's tool_call_id, so follow-up turns break as soon as id != call_id.
Suggested fix
- ResponseInputOutputItem::CustomToolCall {
- id,
- name,
- input,
- output,
- ..
- } => {
+ ResponseInputOutputItem::CustomToolCall {
+ call_id,
+ name,
+ input,
+ output,
+ ..
+ } => {
// Custom tool call from history - convert to function tool call format
messages.push(ChatMessage::Assistant {
content: None,
name: None,
tool_calls: Some(vec![ToolCall {
- id: id.clone(),
+ id: call_id.clone(),
tool_type: "function".to_string(),
function: FunctionCallResponse {
name: name.clone(),
arguments: Some(input.clone()),
},
}]),
reasoning_content: None,
});
if let Some(output_text) = output {
messages.push(ChatMessage::Tool {
content: MessageContent::Text(output_text.clone()),
- tool_call_id: id.clone(),
+ tool_call_id: call_id.clone(),
});
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ResponseInputOutputItem::CustomToolCall { | |
| id, | |
| name, | |
| input, | |
| output, | |
| .. | |
| } => { | |
| // Custom tool call from history - convert to function tool call format | |
| messages.push(ChatMessage::Assistant { | |
| content: None, | |
| name: None, | |
| tool_calls: Some(vec![ToolCall { | |
| id: id.clone(), | |
| tool_type: "function".to_string(), | |
| function: FunctionCallResponse { | |
| name: name.clone(), | |
| arguments: Some(input.clone()), | |
| }, | |
| }]), | |
| reasoning_content: None, | |
| }); | |
| if let Some(output_text) = output { | |
| messages.push(ChatMessage::Tool { | |
| content: MessageContent::Text(output_text.clone()), | |
| tool_call_id: id.clone(), | |
| }); | |
| } | |
| } | |
| ResponseInputOutputItem::CustomToolCallOutput { | |
| call_id, output, .. | |
| } => { | |
| messages.push(ChatMessage::Tool { | |
| content: MessageContent::Text(output.clone()), | |
| tool_call_id: call_id.clone(), | |
| }); | |
| } | |
| ResponseInputOutputItem::CustomToolCall { | |
| call_id, | |
| name, | |
| input, | |
| output, | |
| .. | |
| } => { | |
| // Custom tool call from history - convert to function tool call format | |
| messages.push(ChatMessage::Assistant { | |
| content: None, | |
| name: None, | |
| tool_calls: Some(vec![ToolCall { | |
| id: call_id.clone(), | |
| tool_type: "function".to_string(), | |
| function: FunctionCallResponse { | |
| name: name.clone(), | |
| arguments: Some(input.clone()), | |
| }, | |
| }]), | |
| reasoning_content: None, | |
| }); | |
| if let Some(output_text) = output { | |
| messages.push(ChatMessage::Tool { | |
| content: MessageContent::Text(output_text.clone()), | |
| tool_call_id: call_id.clone(), | |
| }); | |
| } | |
| } | |
| ResponseInputOutputItem::CustomToolCallOutput { | |
| call_id, output, .. | |
| } => { | |
| messages.push(ChatMessage::Tool { | |
| content: MessageContent::Text(output.clone()), | |
| tool_call_id: call_id.clone(), | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model_gateway/src/routers/grpc/regular/responses/conversions.rs` around lines
136 - 172, The assistant tool-call replay uses the wrapper conversation item id
(`id`) instead of the backend tool identifier (`call_id`), causing mismatched
tool_call.id vs tool message tool_call_id; update the
ResponseInputOutputItem::CustomToolCall branch to use the actual backend call id
(call_id) for the ToolCall.id and ensure the Tool message uses the same call_id
(and likewise ensure ResponseInputOutputItem::CustomToolCallOutput uses call_id
for tool_call_id), i.e., replace usages of id.clone() with the backend call
identifier so ToolCall { id: ... } and ChatMessage::Tool { tool_call_id: ... }
consistently use call_id.
| // Add tool calls (convert custom tools from FunctionToolCall to CustomToolCall) | ||
| let custom_tool_names: std::collections::HashSet<&str> = self | ||
| .original_request | ||
| .tools | ||
| .as_ref() | ||
| .map(|tools| { | ||
| tools | ||
| .iter() | ||
| .filter_map(|t| match t { | ||
| ResponseTool::Custom(ct) => Some(ct.name.as_str()), | ||
| _ => None, | ||
| }) | ||
| .collect() | ||
| }) | ||
| .unwrap_or_default(); | ||
|
|
||
| for tc in self.tool_calls { | ||
| match tc { | ||
| ResponseOutputItem::FunctionToolCall { | ||
| id, | ||
| call_id, | ||
| name, | ||
| arguments, | ||
| output: tc_output, | ||
| status, | ||
| } if custom_tool_names.contains(name.as_str()) => { | ||
| output.push(ResponseOutputItem::CustomToolCall { | ||
| id, | ||
| call_id, | ||
| name, | ||
| input: arguments, | ||
| output: tc_output, | ||
| status, | ||
| }); | ||
| } | ||
| other => output.push(other), | ||
| } | ||
| } |
There was a problem hiding this comment.
The new streamed CustomToolCall items will expose call_id: "".
Line 410 copies call_id from the accumulated FunctionToolCall, but process_chunk() only fills id, name, and arguments for streamed tool-call deltas and leaves call_id at its String::new() initializer. The non-MCP streaming path will therefore return custom_tool_call items with an empty call_id, so clients have nothing usable to echo back in custom_tool_call_output.
Please populate call_id from the streamed tool-call id before this conversion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model_gateway/src/routers/grpc/regular/responses/streaming.rs` around lines
382 - 419, The conversion to CustomToolCall is producing empty call_id because
streamed deltas only set the streamed tool-call id in the FunctionToolCall.id
field (process_chunk() fills id, name, arguments but leaves call_id empty);
update the conversion loop that iterates self.tool_calls (in streaming.rs) so
that when matching ResponseOutputItem::FunctionToolCall you populate call_id
from the FunctionToolCall.id (or from the streamed tool-call id field) before
constructing ResponseOutputItem::CustomToolCall; ensure you copy id -> call_id
(or use a non-empty call identifier derived from id) so CustomToolCall.call_id
is populated for downstream consumers.
| FunctionCallEvent::ARGUMENTS_DONE => { | ||
| parsed_data["type"] = json!(McpEvent::CALL_ARGUMENTS_DONE); | ||
| // Check if this belongs to a custom tool by looking up the tool name | ||
| let is_custom = parsed_data | ||
| .get("name") | ||
| .and_then(|v| v.as_str()) | ||
| .is_some_and(|name| { | ||
| ctx.original_request.tools.as_ref().is_some_and(|tools| { | ||
| tools | ||
| .iter() | ||
| .any(|t| matches!(t, ResponseTool::Custom(ct) if ct.name == name)) | ||
| }) | ||
| }); | ||
|
|
||
| if is_custom { | ||
| parsed_data["type"] = json!(CustomToolCallEvent::INPUT_DONE); | ||
|
|
||
| // Rename arguments → input | ||
| if let Some(args) = parsed_data | ||
| .as_object_mut() | ||
| .and_then(|o| o.remove("arguments")) | ||
| { | ||
| parsed_data["input"] = args; | ||
| } | ||
|
|
||
| // Transform item_id from fc_* to mcp_* | ||
| if let Some(item_id) = parsed_data.get("item_id").and_then(|v| v.as_str()) { | ||
| if let Some(stripped) = item_id.strip_prefix("fc_") { | ||
| let new_id = format!("mcp_{stripped}"); | ||
| parsed_data["item_id"] = json!(new_id); | ||
| // Transform item_id from fc_* to ctc_* | ||
| if let Some(item_id) = parsed_data.get("item_id").and_then(|v| v.as_str()) { | ||
| if let Some(stripped) = item_id.strip_prefix("fc_") { | ||
| parsed_data["item_id"] = json!(format!("ctc_{stripped}")); | ||
| } | ||
| } | ||
| } else { | ||
| parsed_data["type"] = json!(McpEvent::CALL_ARGUMENTS_DONE); | ||
|
|
||
| // Transform item_id from fc_* to mcp_* | ||
| if let Some(item_id) = parsed_data.get("item_id").and_then(|v| v.as_str()) { | ||
| if let Some(stripped) = item_id.strip_prefix("fc_") { | ||
| let new_id = format!("mcp_{stripped}"); | ||
| parsed_data["item_id"] = json!(new_id); | ||
| } | ||
| } |
There was a problem hiding this comment.
Custom-tool input.done never survives this transform.
Line 202 tries to re-identify the tool from parsed_data["name"], but response.function_call_arguments.done does not carry the tool name in the event shape used elsewhere in this stack. is_custom therefore stays false, so custom calls in the interception path still fall into the MCP branch and get mcp_* ids. Even if the payload type were rewritten here, forward_streaming_event() still derives the SSE header from map_event_name(evt), which hard-maps ARGUMENTS_DONE to response.mcp_call_arguments.done. That leaves custom_tool_call output items paired with MCP argument events.
Please carry the tool kind forward from the earlier buffered call metadata keyed by item_id/output_index, and use that for both the buffered delta and the done event.
| ResponseTool::WebSearchPreview(_) => serde_json::to_value(tool).ok(), | ||
| ResponseTool::CodeInterpreter(_) => serde_json::to_value(tool).ok(), | ||
| ResponseTool::Function(_) => None, | ||
| ResponseTool::Custom(_) => None, |
There was a problem hiding this comment.
Restore custom tools in the final response payload.
This helper is used right before the OpenAI streaming/non-streaming paths return and persist the final response. Returning None for ResponseTool::Custom means any response that also restores MCP/builtin tools overwrites response.tools without the original custom entry.
💡 Suggested fix
- ResponseTool::Custom(_) => None,
+ ResponseTool::Custom(_) => serde_json::to_value(tool).ok(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model_gateway/src/routers/openai/responses/utils.rs` at line 234, The match
arm for ResponseTool::Custom currently returns None which causes custom tools to
be dropped when restoring final response tools; update the ResponseTool handling
so that ResponseTool::Custom(_) returns Some(...) preserving the custom tool
entry (i.e., return the custom tool value instead of None) so that restoring
MCP/builtin tools merges with rather than overwrites response.tools; locate the
match that contains ResponseTool::Custom and change its branch to return the
appropriate Some variant that contains the custom tool.
| /// Test custom tool input item validation - empty output rejected | ||
| #[test] | ||
| fn test_validate_custom_tool_call_output_empty() { | ||
| let request = ResponsesRequest { | ||
| input: ResponseInput::Items(vec![ResponseInputOutputItem::CustomToolCallOutput { | ||
| id: Some("ctc_123".to_string()), | ||
| call_id: "call_1".to_string(), | ||
| output: String::new(), | ||
| status: None, | ||
| }]), | ||
| ..Default::default() | ||
| }; | ||
| let result = request.validate(); | ||
| assert!( | ||
| result.is_err(), | ||
| "CustomToolCallOutput with empty output should be invalid" | ||
| ); |
There was a problem hiding this comment.
This test can pass before the empty-output validator is exercised.
ResponsesRequest::validate() already rejects input: Items([...]) when there is no message item, so this fixture can stay green even if the CustomToolCallOutput.output.is_empty() check regresses. Add a valid message item or assert the specific field/code you expect from the empty-output rule.
Suggested fixture change
let request = ResponsesRequest {
- input: ResponseInput::Items(vec![ResponseInputOutputItem::CustomToolCallOutput {
- id: Some("ctc_123".to_string()),
- call_id: "call_1".to_string(),
- output: String::new(),
- status: None,
- }]),
+ input: ResponseInput::Items(vec![
+ ResponseInputOutputItem::SimpleInputMessage {
+ content: StringOrContentParts::String("run the tool".to_string()),
+ role: "user".to_string(),
+ r#type: None,
+ },
+ ResponseInputOutputItem::CustomToolCallOutput {
+ id: Some("ctc_123".to_string()),
+ call_id: "call_1".to_string(),
+ output: String::new(),
+ status: None,
+ },
+ ]),
..Default::default()
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Test custom tool input item validation - empty output rejected | |
| #[test] | |
| fn test_validate_custom_tool_call_output_empty() { | |
| let request = ResponsesRequest { | |
| input: ResponseInput::Items(vec![ResponseInputOutputItem::CustomToolCallOutput { | |
| id: Some("ctc_123".to_string()), | |
| call_id: "call_1".to_string(), | |
| output: String::new(), | |
| status: None, | |
| }]), | |
| ..Default::default() | |
| }; | |
| let result = request.validate(); | |
| assert!( | |
| result.is_err(), | |
| "CustomToolCallOutput with empty output should be invalid" | |
| ); | |
| /// Test custom tool input item validation - empty output rejected | |
| #[test] | |
| fn test_validate_custom_tool_call_output_empty() { | |
| let request = ResponsesRequest { | |
| input: ResponseInput::Items(vec![ | |
| ResponseInputOutputItem::SimpleInputMessage { | |
| content: StringOrContentParts::String("run the tool".to_string()), | |
| role: "user".to_string(), | |
| r#type: None, | |
| }, | |
| ResponseInputOutputItem::CustomToolCallOutput { | |
| id: Some("ctc_123".to_string()), | |
| call_id: "call_1".to_string(), | |
| output: String::new(), | |
| status: None, | |
| }, | |
| ]), | |
| ..Default::default() | |
| }; | |
| let result = request.validate(); | |
| assert!( | |
| result.is_err(), | |
| "CustomToolCallOutput with empty output should be invalid" | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model_gateway/tests/spec/responses.rs` around lines 1521 - 1537, The test
test_validate_custom_tool_call_output_empty is currently passing due to
ResponsesRequest::validate rejecting Items when there is no Message item, so
update the fixture to ensure the empty-output validator is actually exercised:
construct a ResponsesRequest whose input contains both the
ResponseInputOutputItem::CustomToolCallOutput with output set to empty AND a
valid message item (e.g., a ResponseInput::Message entry) so validate() advances
to the CustomToolCallOutput check, then assert the specific validation error for
CustomToolCallOutput.output (or an expected error code/message) instead of just
is_err(); reference ResponsesRequest::validate, ResponseInput::Items, and
ResponseInputOutputItem::CustomToolCallOutput.output when making the change.
1 participant
Description
Feature Request: #998
Problem
OpenAI's Responses API supports a
customtool type that allows models to return arbitrary text input (not JSON) to client-defined tools, with optional grammar constraints (Lark CFG or Regex). SMG currently only supportsfunction,web_search_preview,code_interpreter, andmcptool types, so requests with"type": "custom"tools are rejected or unrecognized.Solution
Add end-to-end
CustomToolCallsupport to the OpenAI Responses API pipeline. Custom tools are sent to backends as function tools (with empty parameters), and responses are transformed back to thecustom_tool_callformat withctc_ID prefix andinputfield (instead ofarguments). This implementationsupports both HTTP proxy (cloud OpenAI) and gRPC (self-hosted SGLang/vLLM) backends.
## Changes
Protocol Types (
crates/protocols/src/):Custom(CustomTool)variant toResponseToolenum withCustomToolandCustomToolFormatstructsCustomToolCallandCustomToolCallOutputvariants toResponseInputOutputItemandResponseOutputItemCustomToolCallEvent(input delta/done) andItemType::CustomToolCallto event typesis_custom_tool_call_type()helper functionID Generation & Persistence (
crates/data_connector/,model_gateway/src/routers/persistence_utils.rs):"custom_tool_call" => "ctc"prefix mapping inmake_item_id()ITEM_TYPE_FIELDSentries and whole-item storage for custom tool call typesBackend Conversion (
model_gateway/src/routers/grpc/):extract_tools_from_response_tools()converts custom tools to function tools (empty params) for backendsToolLikeimpl handlesCustomvariant for Harmony builderbuild_tool_response()andchat_to_responses()emitCustomToolCalloutput itemsCustomToolCall/CustomToolCallOutputinput items in conversation historyStreaming (
model_gateway/src/routers/openai/responses/,grpc/common/responses/streaming.rs):function_call→custom_tool_callin streaming events, renamearguments→input, transformfc_→ctc_ID prefixemit_custom_tool_call_input_delta/done()methods to event emitterCustomToolCalltoOutputItemTypewithctc_prefixHistory (
model_gateway/src/routers/openai/responses/history.rs):custom_tool_callandcustom_tool_call_outputitems from conversation storageUnit Tests (30 new tests):
ctc_prefix generationchat_to_responsesoutputTest Plan
Unit tests: