From e0313ab8f3d70ec7eb7676de44037c5df2f56176 Mon Sep 17 00:00:00 2001 From: oudecheng <13802883547@139.com> Date: Wed, 12 Aug 2026 15:37:51 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E5=A2=9E=E5=BC=BA=20panic=20=E5=AE=89?= =?UTF-8?q?=E5=85=A8=E4=B8=8E=E8=AE=A1=E7=AE=97=E5=99=A8=E5=81=A5=E5=A3=AE?= =?UTF-8?q?=E6=80=A7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - agent_loop/processor: catch_unwind 隔离 panic，防止消息静默丢失 - calculator: 拒绝 NaN/Infinity 输入，修复阶乘溢出（上限 34），拒绝非有限表达式结果 - message: 修复 sanitize 两阶段删除索引未排序导致的越界 panic - utils: 新增 panic_payload_message 提取可读 panic 消息 - .gitignore: 忽略 artifacts/ 测试产物目录 --- docs/CHANGELOG.md | 13 +++- src/agent/agent_loop.rs | 161 +++++++++++++++++++++++++++++++++++---- src/bus/message.rs | 7 +- src/gateway/processor.rs | 29 +++++-- src/tools/calculator.rs | 133 ++++++++++++++++++++++++++++++-- src/utils.rs | 38 +++++++++ 6 files changed, 352 insertions(+), 29 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 132ebea..90ad17f 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,7 +4,7 @@ ## [0.3.5] - 2026-08-12 -较 [0.3.4] 的 5 个 commit 迭代，聚焦 **可观测性**、**并发性能** 与 **外部集成健壮性** 三大方向。 +较 [0.3.4] 的 7 个 commit 迭代，聚焦 **可观测性**、**并发性能** 与 **外部集成健壮性** 三大方向。 ### 新增功能 @@ -43,6 +43,17 @@ - `ChatContainer`：新增 `topicId` prop 透传给 `MessageInput`。 - `MessageInput`：监听 `topicId` 变化清空草稿，替代原 key remount 重置机制。 +#### Panic 安全增强 +- `agent_loop.rs` / `processor.rs`：用 `catch_unwind` 隔离工具执行和消息处理的 panic，归一化为错误返回，LLM 可见错误并自我纠正，防止用户消息被静默丢弃。 +- `utils.rs`：新增 `panic_payload_message()` 从 panic payload 中提取可读消息，支持 `&str` / `String` / 其他类型降级。 +- `bus/message.rs`：修复 `sanitize_incomplete_tool_call_sequences` 两阶段删除索引未全局排序导致的越界 panic（Phase 1 降序 + Phase 1.5 升序 → 合并后必须重新排序）。 + +#### 计算器健壮性 +- 拒绝 `"NaN"` / `"inf"` 等非有限输入，防止 `sort_by` 中 `partial_cmp().unwrap()` panic。 +- 修复阶乘溢出：上限从 170 降至 34（35! 超出 `u128::MAX`），改用 `checked_mul` 替代 unchecked 乘法。 +- `evaluate` 表达式拒绝非有限结果（如 `1/0` → inf、`0/0` → NaN）。 +- 新增 7 个 tokio 测试覆盖所有边界场景。 + ### 测试 - MCP 超时路径新增 2 个 tokio 单元测试。 diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs index 13e9900..48a8301 100644 --- a/src/agent/agent_loop.rs +++ b/src/agent/agent_loop.rs @@ -15,6 +15,7 @@ use crate::text::{char_count, take_prefix_chars, take_suffix_chars}; use crate::tools::{ToolContext, ToolRegistry}; use crate::utils::format_error_chain; use async_trait::async_trait; +use futures_util::FutureExt; use std::borrow::Cow; use std::collections::{HashMap, VecDeque}; use std::hash::{Hash, Hasher}; @@ -2120,18 +2121,36 @@ impl AgentLoop { } }; - match tool - .execute_with_context( - &{ - let mut ctx = self.tool_context.clone(); - ctx.tool_call_id = Some(tool_call.id.clone()); - ctx - }, - normalized_arguments.clone(), - ) - .await - { - Ok(result) => { + let tool_context = { + let mut ctx = self.tool_context.clone(); + ctx.tool_call_id = Some(tool_call.id.clone()); + ctx + }; + // catch_unwind 隔离单个工具的 panic：否则一个工具崩溃会终止整个 turn， + // 用户消息被静默丢弃。归一化为工具级失败后 LLM 还能看到错误并自我纠正。 + let execution = std::panic::AssertUnwindSafe( + tool.execute_with_context(&tool_context, normalized_arguments.clone()), + ) + .catch_unwind() + .await; + + match execution { + Err(payload) => { + let error = format!( + "Tool '{}' panicked: {}", + tool_call.name, + crate::utils::panic_payload_message(&payload) + ); + tracing::error!( + tool = %tool_call.name, + args = %truncate_args(&tool_call.arguments, 4_000), + normalized_args = %truncate_args(&normalized_arguments, 4_000), + error = %error, + "Tool execution panicked" + ); + ToolExecutionOutcome::failure(format!("Error: {}", error), Some(error)) + } + Ok(Ok(result)) => { if result.success { if let Some(pending_output) = parse_pending_tool_output(&result.output) { ToolExecutionOutcome::pending(pending_output) @@ -2158,7 +2177,7 @@ impl AgentLoop { ToolExecutionOutcome::failure(failure_output, Some(error)) } } - Err(e) => { + Ok(Err(e)) => { tracing::error!( tool = %tool_call.name, args = %truncate_args(&tool_call.arguments, 4_000), @@ -3111,6 +3130,122 @@ mod tests { assert_eq!(messages.len(), 3); } + /// 良构不变量校验：sanitize 的输出必须满足 + /// 1. 每个带 tool_calls 的 assistant 之后紧邻其全部 tool 结果（无其他消息隔断）； + /// 2. 每个 tool 消息都有存活的父 assistant。 + fn assert_well_formed(messages: &[ChatMessage]) { + let mut pending: Vec = Vec::new(); + for (i, m) in messages.iter().enumerate() { + if m.role == "assistant" { + if let Some(calls) = m.tool_calls.as_ref().filter(|c| !c.is_empty()) { + assert!( + pending.is_empty(), + "assistant at {i} starts tool_calls while previous results are pending" + ); + pending = calls.iter().map(|tc| tc.id.clone()).collect(); + } + } else if m.role == "tool" { + let tc_id = m.tool_call_id.clone().unwrap_or_default(); + let pos = pending + .iter() + .position(|id| *id == tc_id) + .unwrap_or_else(|| panic!("tool at {i} has no pending parent (id={tc_id})")); + pending.remove(pos); + } else if !pending.is_empty() { + panic!("non-tool message at {i} interrupts pending tool results"); + } + } + assert!( + pending.is_empty(), + "trailing assistant tool_calls without results" + ); + } + + #[test] + fn test_sanitize_mixed_removal_order_does_not_panic_or_corrupt() { + // Phase 1（反向扫描）按降序收集孤儿 assistant 索引 [2,1,0]， + // Phase 1.5（正向扫描）随后按升序追加索引 3 → remove_indices=[2,1,0,3]。 + // 若不全局排序就逐个 Vec::remove，第 4 次删除时越界 panic。 + let mut messages = vec![ + ChatMessage::assistant_with_tool_calls( + "orphan 1", + vec![ToolCall { + id: "call_x".to_string(), + name: "bash".to_string(), + arguments: serde_json::json!({}), + }], + ), + ChatMessage::assistant_with_tool_calls( + "orphan 2", + vec![ToolCall { + id: "call_y".to_string(), + name: "bash".to_string(), + arguments: serde_json::json!({}), + }], + ), + ChatMessage::assistant_with_tool_calls( + "orphan 3", + vec![ToolCall { + id: "call_z".to_string(), + name: "bash".to_string(), + arguments: serde_json::json!({}), + }], + ), + ChatMessage::assistant_with_tool_calls( + "resolved but interrupted", + vec![ToolCall { + id: "call_w".to_string(), + name: "bash".to_string(), + arguments: serde_json::json!({}), + }], + ), + ChatMessage::user("next question"), + ChatMessage::tool("call_w", "bash", "result"), + ]; + + let removed = crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages); + + assert_eq!(removed, 5, "4 assistants + 1 orphaned tool result"); + assert_eq!(messages.len(), 1); + assert_eq!(messages[0].role, "user"); + assert_eq!(messages[0].content, "next question"); + assert_well_formed(&messages); + } + + #[test] + fn test_sanitize_mixed_removal_order_deletes_correct_messages() { + // remove_indices=[0(Phase 1), 1(Phase 1.5)]：先 remove(0) 后索引漂移， + // 未排序时 remove(1) 会误删 user 消息而非第二个 assistant。 + let mut messages = vec![ + ChatMessage::assistant_with_tool_calls( + "orphan", + vec![ToolCall { + id: "call_x".to_string(), + name: "bash".to_string(), + arguments: serde_json::json!({}), + }], + ), + ChatMessage::assistant_with_tool_calls( + "resolved but interrupted", + vec![ToolCall { + id: "call_y".to_string(), + name: "bash".to_string(), + arguments: serde_json::json!({}), + }], + ), + ChatMessage::user("interrupting question"), + ChatMessage::tool("call_y", "bash", "result"), + ]; + + let removed = crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages); + + assert_eq!(removed, 3, "2 assistants + 1 orphaned tool result"); + assert_eq!(messages.len(), 1); + assert_eq!(messages[0].role, "user"); + assert_eq!(messages[0].content, "interrupting question"); + assert_well_formed(&messages); + } + // ===== LLM 重试机制测试 ===== #[test] diff --git a/src/bus/message.rs b/src/bus/message.rs index b60d08b..548120e 100644 --- a/src/bus/message.rs +++ b/src/bus/message.rs @@ -443,7 +443,12 @@ pub(crate) fn sanitize_incomplete_tool_call_sequences(messages: &mut Vec {} + Ok(Err(e)) => { + tracing::error!( + error = %crate::utils::format_error_chain(&e), + "Message processing failed" + ); + crate::observability::metrics::record_message_processing_error(); + } + Err(payload) => { + tracing::error!( + error = %crate::utils::panic_payload_message(&payload), + "Message processing panicked" + ); + crate::observability::metrics::record_message_processing_error(); + } } }, ), diff --git a/src/tools/calculator.rs b/src/tools/calculator.rs index 90d43dd..3a9ad33 100644 --- a/src/tools/calculator.rs +++ b/src/tools/calculator.rs @@ -158,14 +158,20 @@ fn extract_f64(args: &serde_json::Value, key: &str, name: &str) -> Result Err(format!("Missing required parameter: {name}")), Some(v) => { - if let Some(n) = v.as_f64() { - Ok(n) + let n = if let Some(n) = v.as_f64() { + n } else if let Some(s) = v.as_str() { s.parse::() - .map_err(|_| format!("{name} is not a valid number: {s}")) + .map_err(|_| format!("{name} is not a valid number: {s}"))? } else { - Err(format!("{name} must be a number")) + return Err(format!("{name} must be a number")); + }; + // f64::from_str 接受 "NaN"/"inf"；非有限值会使 sort_by 的 + // partial_cmp().unwrap() panic，且算术结果无意义，统一在边界拒绝。 + if !n.is_finite() { + return Err(format!("{name} must be a finite number")); } + Ok(n) } } } @@ -207,6 +213,11 @@ fn extract_values(args: &serde_json::Value, min_len: usize) -> Result, } else { return Err(format!("values[{i}] is not a valid number")); }; + // f64::from_str 接受 "NaN"/"inf"；非有限值会使 sort_by 的 + // partial_cmp().unwrap() panic，且统计结果无意义，统一在边界拒绝。 + if !n.is_finite() { + return Err(format!("values[{i}] is not a finite number")); + } nums.push(n); } Ok(nums) @@ -251,12 +262,16 @@ fn calc_factorial(args: &serde_json::Value) -> Result { } #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] let n = x.round() as u128; - if n > 170 { - return Err("Factorial result exceeds f64 range (max input: 170)".to_string()); + // u128::MAX ≈ 3.4e38，34! 是最后一个不溢出的阶乘（35! ≈ 1.03e40）。 + // 修复前按 f64 范围放行到 170，实际在 n≥35 时 debug panic / release 静默回绕。 + if n > 34 { + return Err("Factorial result exceeds supported integer range (max input: 34)".to_string()); } let mut result: u128 = 1; for i in 2..=n { - result *= i; + result = result + .checked_mul(i) + .ok_or_else(|| "Factorial result exceeds supported integer range".to_string())?; } Ok(result.to_string()) } @@ -412,8 +427,15 @@ fn calc_evaluate(args: &serde_json::Value) -> Result { .ok_or_else(|| "Missing required parameter: expression".to_string())?; meval::eval_str(expression) - .map(format_num) .map_err(|e| format!("Expression evaluation error: {e}")) + .and_then(|n| { + // 表达式可产生非有限结果（如 "1/0" → inf、"0/0" → NaN）， + // 与 extract_values/extract_f64 的边界策略保持一致：拒绝输出。 + if !n.is_finite() { + return Err(format!("Expression result is not a finite number: {expression}")); + } + Ok(format_num(n)) + }) } #[cfg(test)] @@ -779,4 +801,99 @@ mod tests { .contains("Missing required parameters") ); } + + #[tokio::test] + async fn test_median_rejects_nan_string_value() { + // f64::from_str accepts "NaN"; partial_cmp on NaN is None and would + // panic inside sort_by — must surface as a tool error instead. + let tool = CalculatorTool::new(); + let result = tool + .execute(json!({"function": "median", "values": ["NaN", 1.0, 2.0]})) + .await + .unwrap(); + assert!(!result.success); + assert!( + result.error.as_ref().unwrap().contains("finite"), + "expected finiteness error, got: {:?}", + result.error + ); + } + + #[tokio::test] + async fn test_percentile_rejects_infinity_string_value() { + let tool = CalculatorTool::new(); + let result = tool + .execute(json!({"function": "percentile", "values": ["inf", 1.0], "p": 50})) + .await + .unwrap(); + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("finite")); + } + + #[tokio::test] + async fn test_sum_rejects_nan_instead_of_returning_nan() { + let tool = CalculatorTool::new(); + let result = tool + .execute(json!({"function": "sum", "values": ["NaN", 1.0]})) + .await + .unwrap(); + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("finite")); + } + + #[tokio::test] + async fn test_clamp_rejects_non_finite_scalar() { + let tool = CalculatorTool::new(); + let result = tool + .execute(json!({"function": "clamp", "x": "NaN", "min_val": 0.0, "max_val": 1.0})) + .await + .unwrap(); + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("finite")); + } + + #[tokio::test] + async fn test_factorial_large_input_returns_error_not_overflow() { + // 35! ≈ 1.03e40 超出 u128::MAX ≈ 3.4e38：修复前 debug 下乘法溢出 panic、 + // release 下静默回绕。必须返回工具错误。 + let tool = CalculatorTool::new(); + let result = tool + .execute(json!({"function": "factorial", "x": 35.0})) + .await + .unwrap(); + assert!(!result.success); + assert!( + result.error.as_ref().unwrap().contains("range"), + "expected range error, got: {:?}", + result.error + ); + // 34! 仍可精确计算 + let ok = tool + .execute(json!({"function": "factorial", "x": 34.0})) + .await + .unwrap(); + assert!(ok.success); + assert_eq!( + ok.output, + "295232799039604140847618609643520000000" + ); + } + + #[tokio::test] + async fn test_evaluate_rejects_non_finite_result() { + let tool = CalculatorTool::new(); + let division_by_zero = tool + .execute(json!({"function": "evaluate", "expression": "1/0"})) + .await + .unwrap(); + assert!(!division_by_zero.success); + assert!(division_by_zero.error.as_ref().unwrap().contains("finite")); + + let nan_result = tool + .execute(json!({"function": "evaluate", "expression": "0/0"})) + .await + .unwrap(); + assert!(!nan_result.success); + assert!(nan_result.error.as_ref().unwrap().contains("finite")); + } } diff --git a/src/utils.rs b/src/utils.rs index e3b9ba5..ec98538 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -11,6 +11,20 @@ pub fn current_timestamp() -> i64 { .as_millis() as i64 } +/// 从 `catch_unwind` 的 panic payload 中提取可读消息。 +/// +/// `panic!` 的 payload 通常是 `&str` 或 `String`；其他类型（如直接 +/// `panic!(42)`）无法还原原文，返回占位描述。 +pub fn panic_payload_message(payload: &(dyn std::any::Any + Send)) -> String { + if let Some(s) = payload.downcast_ref::<&str>() { + return (*s).to_string(); + } + if let Some(s) = payload.downcast_ref::() { + return s.clone(); + } + "".to_string() +} + /// 递归展开 `error.source()` 链，生成 `"顶层错误\ncaused by: 原因\ncaused by: ..."` 格式的字符串。 pub fn format_error_chain(error: &(dyn std::error::Error + 'static)) -> String { let mut details = vec![error.to_string()]; @@ -23,3 +37,27 @@ pub fn format_error_chain(error: &(dyn std::error::Error + 'static)) -> String { details.join("\ncaused by: ") } + +#[cfg(test)] +mod tests { + use super::*; + use std::any::Any; + + #[test] + fn test_panic_payload_message_str() { + let payload: Box = Box::new("boom"); + assert_eq!(panic_payload_message(&*payload), "boom"); + } + + #[test] + fn test_panic_payload_message_string() { + let payload: Box = Box::new(String::from("boom")); + assert_eq!(panic_payload_message(&*payload), "boom"); + } + + #[test] + fn test_panic_payload_message_other_type() { + let payload: Box = Box::new(42u32); + assert!(panic_payload_message(&*payload).contains("non-string")); + } +}