Skip to content

Commit 115c61b

Browse files
1jehuang1jehuang
committed
Validate live tool call arguments
1 parent 9abbc75 commit 115c61b

7 files changed

Lines changed: 143 additions & 34 deletions

File tree

‎crates/jcode-message-types/src/lib.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,22 @@ impl ToolCall {
2222
Self::normalize_input_to_object(input.clone())
2323
}
2424

25+
pub fn validation_error(&self) -> Option<String> {
26+
if self.name.trim().is_empty() {
27+
return Some("Invalid tool call: tool name must not be empty.".to_string());
28+
}
29+
30+
if !self.input.is_object() {
31+
return Some(format!(
32+
"Invalid tool call for '{}': arguments must be a JSON object, got {}.",
33+
self.name,
34+
json_value_kind(&self.input)
35+
));
36+
}
37+
38+
None
39+
}
40+
2541
pub fn intent_from_input(input: &serde_json::Value) -> Option<String> {
2642
input
2743
.get("intent")
@@ -36,6 +52,17 @@ impl ToolCall {
3652
}
3753
}
3854

55+
fn json_value_kind(value: &serde_json::Value) -> &'static str {
56+
match value {
57+
serde_json::Value::Null => "null",
58+
serde_json::Value::Bool(_) => "boolean",
59+
serde_json::Value::Number(_) => "number",
60+
serde_json::Value::String(_) => "string",
61+
serde_json::Value::Array(_) => "array",
62+
serde_json::Value::Object(_) => "object",
63+
}
64+
}
65+
3966
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize, PartialEq, Eq)]
4067
pub struct InputShellResult {
4168
pub command: String,

‎src/agent/turn_loops.rs‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,9 @@ impl Agent {
227227
StreamEvent::ToolUseEnd => {
228228
if let Some(mut tool) = current_tool.take() {
229229
// Parse the accumulated JSON
230-
let tool_input = ToolCall::normalize_input_to_object(
230+
let tool_input =
231231
serde_json::from_str::<serde_json::Value>(&current_tool_input)
232-
.unwrap_or(serde_json::Value::Null),
233-
);
232+
.unwrap_or(serde_json::Value::Null);
234233
tool.input = tool_input.clone();
235234
tool.intent = ToolCall::intent_from_input(&tool_input);
236235

@@ -647,12 +646,37 @@ impl Agent {
647646
// Execute tools and add results
648647
let mut tool_results_dirty = false;
649648
for tc in tool_calls {
650-
self.validate_tool_allowed(&tc.name)?;
651-
652649
let message_id = assistant_message_id
653650
.clone()
654651
.unwrap_or_else(|| self.session.id.clone());
655652

653+
if let Some(error_msg) = tc.validation_error() {
654+
logging::warn(&error_msg);
655+
Bus::global().publish(BusEvent::ToolUpdated(ToolEvent {
656+
session_id: self.session.id.clone(),
657+
message_id: message_id.clone(),
658+
tool_call_id: tc.id.clone(),
659+
tool_name: tc.name.clone(),
660+
status: ToolStatus::Error,
661+
title: None,
662+
}));
663+
if print_output {
664+
println!("\n → {}", error_msg);
665+
}
666+
self.add_message(
667+
Role::User,
668+
vec![ContentBlock::ToolResult {
669+
tool_use_id: tc.id,
670+
content: error_msg,
671+
is_error: Some(true),
672+
}],
673+
);
674+
tool_results_dirty = true;
675+
continue;
676+
}
677+
678+
self.validate_tool_allowed(&tc.name)?;
679+
656680
let is_native_tool = JCODE_NATIVE_TOOLS.contains(&tc.name.as_str());
657681

658682
// Check if SDK already executed this tool

‎src/agent/turn_streaming_broadcast.rs‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -295,10 +295,9 @@ impl Agent {
295295
}
296296
StreamEvent::ToolUseEnd => {
297297
if let Some(mut tool) = current_tool.take() {
298-
tool.input = ToolCall::normalize_input_to_object(
298+
tool.input =
299299
serde_json::from_str::<serde_json::Value>(&current_tool_input)
300-
.unwrap_or(serde_json::Value::Null),
301-
);
300+
.unwrap_or(serde_json::Value::Null);
302301
tool.refresh_intent_from_input();
303302

304303
let _ = event_tx.send(ServerEvent::ToolExec {
@@ -713,12 +712,32 @@ impl Agent {
713712
}
714713
let tc = &tool_calls[tool_index];
715714

716-
self.validate_tool_allowed(&tc.name)?;
717-
718715
let message_id = assistant_message_id
719716
.clone()
720717
.unwrap_or_else(|| self.session.id.clone());
721718

719+
if let Some(error_msg) = tc.validation_error() {
720+
logging::warn(&error_msg);
721+
let _ = event_tx.send(ServerEvent::ToolDone {
722+
id: tc.id.clone(),
723+
name: tc.name.clone(),
724+
output: error_msg.clone(),
725+
error: Some(error_msg.clone()),
726+
});
727+
self.add_message(
728+
Role::User,
729+
vec![ContentBlock::ToolResult {
730+
tool_use_id: tc.id.clone(),
731+
content: error_msg,
732+
is_error: Some(true),
733+
}],
734+
);
735+
tool_results_dirty = true;
736+
continue;
737+
}
738+
739+
self.validate_tool_allowed(&tc.name)?;
740+
722741
let is_native_tool = JCODE_NATIVE_TOOLS.contains(&tc.name.as_str());
723742

724743
// Check if SDK already executed this tool

‎src/agent/turn_streaming_mpsc.rs‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -298,10 +298,9 @@ impl Agent {
298298
}
299299
StreamEvent::ToolUseEnd => {
300300
if let Some(mut tool) = current_tool.take() {
301-
tool.input = ToolCall::normalize_input_to_object(
301+
tool.input =
302302
serde_json::from_str::<serde_json::Value>(&current_tool_input)
303-
.unwrap_or(serde_json::Value::Null),
304-
);
303+
.unwrap_or(serde_json::Value::Null);
305304
tool.refresh_intent_from_input();
306305

307306
let _ = event_tx.send(ServerEvent::ToolExec {
@@ -743,12 +742,32 @@ impl Agent {
743742
}
744743
let tc = &tool_calls[tool_index];
745744

746-
self.validate_tool_allowed(&tc.name)?;
747-
748745
let message_id = assistant_message_id
749746
.clone()
750747
.unwrap_or_else(|| self.session.id.clone());
751748

749+
if let Some(error_msg) = tc.validation_error() {
750+
logging::warn(&error_msg);
751+
let _ = event_tx.send(ServerEvent::ToolDone {
752+
id: tc.id.clone(),
753+
name: tc.name.clone(),
754+
output: error_msg.clone(),
755+
error: Some(error_msg.clone()),
756+
});
757+
self.add_message(
758+
Role::User,
759+
vec![ContentBlock::ToolResult {
760+
tool_use_id: tc.id.clone(),
761+
content: error_msg,
762+
is_error: Some(true),
763+
}],
764+
);
765+
tool_results_dirty = true;
766+
continue;
767+
}
768+
769+
self.validate_tool_allowed(&tc.name)?;
770+
752771
let is_native_tool = JCODE_NATIVE_TOOLS.contains(&tc.name.as_str());
753772

754773
if let Some((sdk_content, sdk_is_error)) = sdk_tool_results.remove(&tc.id) {

‎src/message/tests.rs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,39 @@ fn tool_call_normalizes_non_object_input_to_empty_object() {
8888
);
8989
}
9090

91+
#[test]
92+
fn tool_call_validation_rejects_empty_name_and_non_object_input() {
93+
let empty_name = ToolCall {
94+
id: "call_1".to_string(),
95+
name: "".to_string(),
96+
input: serde_json::json!({}),
97+
intent: None,
98+
};
99+
assert_eq!(
100+
empty_name.validation_error().as_deref(),
101+
Some("Invalid tool call: tool name must not be empty.")
102+
);
103+
104+
let primitive_args = ToolCall {
105+
id: "call_2".to_string(),
106+
name: "read".to_string(),
107+
input: serde_json::json!(20),
108+
intent: None,
109+
};
110+
assert_eq!(
111+
primitive_args.validation_error().as_deref(),
112+
Some("Invalid tool call for 'read': arguments must be a JSON object, got number.")
113+
);
114+
115+
let valid = ToolCall {
116+
id: "call_3".to_string(),
117+
name: "read".to_string(),
118+
input: serde_json::json!({"path":"README.md"}),
119+
intent: None,
120+
};
121+
assert_eq!(valid.validation_error(), None);
122+
}
123+
91124
#[test]
92125
fn sanitize_tool_id_hyphens_passthrough() {
93126
assert_eq!(sanitize_tool_id("call-abc-123"), "call-abc-123");

‎src/provider/openai/stream.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,10 @@ pub(super) struct StreamingToolCallState {
151151
}
152152

153153
fn normalize_openai_tool_arguments(raw_arguments: String) -> String {
154-
let parsed = serde_json::from_str::<Value>(&raw_arguments);
155-
let should_normalize = !matches!(parsed, Ok(Value::Object(_)));
156-
if should_normalize {
154+
if raw_arguments.trim().is_empty() {
157155
let total = NORMALIZED_NULL_TOOL_ARGUMENTS.fetch_add(1, Ordering::Relaxed) + 1;
158156
crate::logging::warn(&format!(
159-
"[openai] Normalized non-object tool arguments to empty object (total={})",
157+
"[openai] Normalized empty tool arguments to empty object (total={})",
160158
total
161159
));
162160
"{}".to_string()

‎src/provider/openrouter_sse_stream.rs‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,6 @@ fn truncated_stream_payload_context(data: &str) -> String {
44
crate::util::truncate_str(&data.trim().replace('\n', "\\n"), 240).to_string()
55
}
66

7-
fn normalize_tool_arguments(raw_arguments: String) -> String {
8-
match serde_json::from_str::<Value>(&raw_arguments) {
9-
Ok(Value::Object(_)) => raw_arguments,
10-
_ => "{}".to_string(),
11-
}
12-
}
13-
147
// ============================================================================
158
// SSE Stream Parser
169
// ============================================================================
@@ -363,15 +356,13 @@ impl OpenRouterStream {
363356
// Emit previous tool call if any
364357
if let Some(prev) = self.current_tool_call.take()
365358
&& !prev.id.is_empty()
366-
&& !prev.name.is_empty()
367359
{
368360
self.pending.push_back(StreamEvent::ToolUseStart {
369361
id: prev.id,
370362
name: prev.name,
371363
});
372-
self.pending.push_back(StreamEvent::ToolInputDelta(
373-
normalize_tool_arguments(prev.arguments),
374-
));
364+
self.pending
365+
.push_back(StreamEvent::ToolInputDelta(prev.arguments));
375366
self.pending.push_back(StreamEvent::ToolUseEnd);
376367
}
377368

@@ -408,15 +399,13 @@ impl OpenRouterStream {
408399
// Emit any pending tool call
409400
if let Some(tc) = self.current_tool_call.take()
410401
&& !tc.id.is_empty()
411-
&& !tc.name.is_empty()
412402
{
413403
self.pending.push_back(StreamEvent::ToolUseStart {
414404
id: tc.id,
415405
name: tc.name,
416406
});
417-
self.pending.push_back(StreamEvent::ToolInputDelta(
418-
normalize_tool_arguments(tc.arguments),
419-
));
407+
self.pending
408+
.push_back(StreamEvent::ToolInputDelta(tc.arguments));
420409
self.pending.push_back(StreamEvent::ToolUseEnd);
421410
}
422411

0 commit comments

Comments
 (0)