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.
This commit is contained in:
leokun
2026-08-27 21:45:36 +08:00
parent c5d578c5b1
commit 9cb16e116b
2 changed files with 174 additions and 3 deletions
+110
View File
@@ -50,6 +50,9 @@ pub(super) async fn start(
return local::subagents_disabled(call); 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() { match normalized(&call.name).as_str() {
"shell" | "read" | "delete" | "grep" | "glob" | "readlints" | "task" | "callmcptool" "shell" | "read" | "delete" | "grep" | "glob" | "readlints" | "task" | "callmcptool"
| "fetchmcpresource" | "getmcptools" => exec::start(runtime, call, context).await, | "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<Option<ToolCall>> {
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 { fn is_mcp_auth(call: &ToolCall) -> bool {
normalized(&call.name) == "callmcptool" normalized(&call.name) == "callmcptool"
&& call && call
@@ -87,3 +139,61 @@ pub(super) fn normalized(name: &str) -> String {
.flat_map(char::to_lowercase) .flat_map(char::to_lowercase)
.collect() .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"
);
}
}
+64 -3
View File
@@ -230,12 +230,27 @@ fn openai_chat_messages(instructions: &str, messages: &[ProjectedMessage]) -> Re
calls, calls,
.. ..
} => { } => {
value.insert("content".into(), Value::String(text.clone()));
let replay_reasoning = replay_state let replay_reasoning = replay_state
.as_ref() .as_ref()
.filter(|state| state.provider_kind == "openai_chat") .filter(|state| state.provider_kind == "openai_chat")
.and_then(|state| state.value.get("reasoning_content")) .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 { if let Some(reasoning) = replay_reasoning {
value.insert("reasoning_content".into(), Value::String(reasoning.into())); value.insert("reasoning_content".into(), Value::String(reasoning.into()));
} }
@@ -398,7 +413,7 @@ mod tests {
model::{ContentPart, ProjectedContent, ProjectedMessage, ToolResultContent}, model::{ContentPart, ProjectedContent, ProjectedMessage, ToolResultContent},
model::{ProviderReplayState, Role, ToolCallContent}, model::{ProviderReplayState, Role, ToolCallContent},
}; };
use serde_json::json; use serde_json::{json, Value};
#[test] #[test]
fn chat_replay_state_is_encoded_as_reasoning_content() { 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"); 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] #[test]
fn another_provider_replay_does_not_invent_chat_reasoning_content() { fn another_provider_replay_does_not_invent_chat_reasoning_content() {
let messages = openai_chat_messages( let messages = openai_chat_messages(