From 9cb16e116ba3dbb71b7502e8075fc2332b45ec5a Mon Sep 17 00:00:00 2001 From: leokun Date: Thu, 27 Aug 2026 21:45:36 +0800 Subject: [PATCH] feat(cursor): normalize block_until_ms argument for shell tool calls - Added a new function to normalize the `block_until_ms` argument for shell tool calls, ensuring it is an integer and within valid range. - Updated the `start` function to utilize the normalization logic. - Introduced tests to validate the behavior of the normalization function, including acceptance of integer-valued floats and rejection of invalid values. --- server/src/cursor/tools/dispatch/mod.rs | 110 ++++++++++++++++++++++++ server/src/provider/openai_chat.rs | 67 ++++++++++++++- 2 files changed, 174 insertions(+), 3 deletions(-) diff --git a/server/src/cursor/tools/dispatch/mod.rs b/server/src/cursor/tools/dispatch/mod.rs index 4cdadd0..39957ba 100644 --- a/server/src/cursor/tools/dispatch/mod.rs +++ b/server/src/cursor/tools/dispatch/mod.rs @@ -50,6 +50,9 @@ pub(super) async fn start( return local::subagents_disabled(call); } + let normalized_call = normalize_block_until_ms(call)?; + let call = normalized_call.as_ref().unwrap_or(call); + match normalized(&call.name).as_str() { "shell" | "read" | "delete" | "grep" | "glob" | "readlints" | "task" | "callmcptool" | "fetchmcpresource" | "getmcptools" => exec::start(runtime, call, context).await, @@ -62,6 +65,55 @@ pub(super) async fn start( } } +fn normalize_block_until_ms(call: &ToolCall) -> Result> { + if normalized(&call.name) != "shell" { + return Ok(None); + } + let Some(value) = call.arguments.get("block_until_ms") else { + return Ok(None); + }; + + let integer = if let Some(value) = value.as_i64() { + value + } else { + let value = value.as_f64().ok_or_else(|| { + Error::Protocol(format!("{} block_until_ms must be an integer", call.name)) + })?; + if !value.is_finite() || value.fract() != 0.0 { + return Err(Error::Protocol(format!( + "{} block_until_ms must be an integer", + call.name + ))); + } + if value < i64::MIN as f64 || value > i64::MAX as f64 { + return Err(Error::Protocol(format!( + "{} block_until_ms is out of range", + call.name + ))); + } + value as i64 + }; + + if integer < 0 { + return Err(Error::Protocol(format!( + "{} block_until_ms is out of range", + call.name + ))); + } + + if value.as_i64().is_some() { + return Ok(None); + } + + let mut normalized_call = call.clone(); + normalized_call + .arguments + .as_object_mut() + .ok_or_else(|| Error::Protocol(format!("{} arguments must be a JSON object", call.name)))? + .insert("block_until_ms".into(), serde_json::Value::from(integer)); + Ok(Some(normalized_call)) +} + fn is_mcp_auth(call: &ToolCall) -> bool { normalized(&call.name) == "callmcptool" && call @@ -87,3 +139,61 @@ pub(super) fn normalized(name: &str) -> String { .flat_map(char::to_lowercase) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + + fn tool(name: &str, arguments: serde_json::Value) -> ToolCall { + ToolCall { + index: 0, + call_id: "call-1".into(), + model_call_id: "model-call-1".into(), + name: name.into(), + arguments_text: arguments.to_string(), + arguments, + } + } + + #[test] + fn shell_accepts_integer_valued_float_timeout() { + let call = tool( + "Shell", + serde_json::json!({"command": "echo ok", "block_until_ms": 45_000.0}), + ); + + let call = normalize_block_until_ms(&call).unwrap().unwrap(); + + assert_eq!(call.arguments["block_until_ms"].as_i64(), Some(45_000)); + } + + #[test] + fn shell_rejects_fractional_timeout() { + let call = tool( + "Shell", + serde_json::json!({"command": "echo ok", "block_until_ms": 30_000.5}), + ); + + let error = normalize_block_until_ms(&call).unwrap_err(); + + assert_eq!( + error.to_string(), + "protocol error: Shell block_until_ms must be an integer" + ); + } + + #[test] + fn shell_rejects_negative_timeout_instead_of_defaulting() { + let call = tool( + "Shell", + serde_json::json!({"command": "echo ok", "block_until_ms": -1}), + ); + + let error = normalize_block_until_ms(&call).unwrap_err(); + + assert_eq!( + error.to_string(), + "protocol error: Shell block_until_ms is out of range" + ); + } +} diff --git a/server/src/provider/openai_chat.rs b/server/src/provider/openai_chat.rs index bea7cb2..6cbc921 100644 --- a/server/src/provider/openai_chat.rs +++ b/server/src/provider/openai_chat.rs @@ -230,12 +230,27 @@ fn openai_chat_messages(instructions: &str, messages: &[ProjectedMessage]) -> Re calls, .. } => { - value.insert("content".into(), Value::String(text.clone())); let replay_reasoning = replay_state .as_ref() .filter(|state| state.provider_kind == "openai_chat") .and_then(|state| state.value.get("reasoning_content")) - .and_then(Value::as_str); + .and_then(Value::as_str) + .filter(|reasoning| !reasoning.is_empty()); + + // Chat Completions rejects an empty assistant content string. Tool-call + // assistant messages use null content, while an assistant with no visible + // content at all does not need to be sent. + if text.is_empty() && calls.is_empty() && replay_reasoning.is_none() { + continue; + } + value.insert( + "content".into(), + if text.is_empty() { + Value::Null + } else { + Value::String(text.clone()) + }, + ); if let Some(reasoning) = replay_reasoning { value.insert("reasoning_content".into(), Value::String(reasoning.into())); } @@ -398,7 +413,7 @@ mod tests { model::{ContentPart, ProjectedContent, ProjectedMessage, ToolResultContent}, model::{ProviderReplayState, Role, ToolCallContent}, }; - use serde_json::json; + use serde_json::{json, Value}; #[test] fn chat_replay_state_is_encoded_as_reasoning_content() { @@ -430,6 +445,52 @@ mod tests { assert_eq!(messages[0]["tool_calls"][0]["id"], "call-1"); } + #[test] + fn chat_tool_call_assistant_uses_null_content() { + let messages = openai_chat_messages( + "", + &[ProjectedMessage { + message_id: "test".into(), + role: Role::Assistant, + content: ProjectedContent::Assistant { + text: String::new(), + thinking: String::new(), + replay_state: None, + calls: vec![ToolCallContent { + index: 0, + call_id: "call-1".into(), + name: "Read".into(), + arguments: json!({"path": "README.md"}), + }], + }, + }], + ) + .unwrap(); + + assert_eq!(messages[0]["content"], Value::Null); + assert!(messages[0]["tool_calls"].is_array()); + } + + #[test] + fn chat_contentless_assistant_is_omitted() { + let messages = openai_chat_messages( + "", + &[ProjectedMessage { + message_id: "test".into(), + role: Role::Assistant, + content: ProjectedContent::Assistant { + text: String::new(), + thinking: String::new(), + replay_state: None, + calls: vec![], + }, + }], + ) + .unwrap(); + + assert!(messages.is_empty()); + } + #[test] fn another_provider_replay_does_not_invent_chat_reasoning_content() { let messages = openai_chat_messages(