diff --git a/AGENTS.md b/AGENTS.md index 1a4c8e9..0366a39 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -86,7 +86,7 @@ Scheduler → SessionManager scheduled execution → AgentLoop → Scheduler del - **WebUI/TUI file transfer** streams bytes over authenticated HTTP and sends only short-lived upload IDs/attachment metadata over WebSocket; messages persist local media paths without guaranteeing later availability, and client responses must never expose those paths - **WebUI authentication** protects every management API and `/ws`; only static pairing assets, public health/status, and pairing submission may bypass device auth. Pair-code issuance requires both a real loopback peer and the filesystem-held admin token; never put bearer tokens in URLs or logs - **Providers** are pure HTTP clients; no bus/session/channel awareness -- **Tools** are executed by `AgentLoop`; they receive raw arguments and return string results +- **Tools** are executed by `AgentLoop`; they receive raw arguments and normally return text. Tools that produce model-consumable media use the structured `execute_with_media` side channel; model capability checks and provider content-block serialization stay outside tools ### Concurrency and Lifecycle Invariants diff --git a/README.md b/README.md index 83e6d25..4e8d3a3 100644 --- a/README.md +++ b/README.md @@ -254,7 +254,7 @@ PicoBot 有两类记忆： | 工具 | 说明 | |------|------| | `calculator` | 数学表达式和统计计算 | -| `file_read` / `file_write` / `file_edit` | 文件读写和编辑 | +| `file_read` / `file_write` / `file_edit` | 文件读写和编辑；`file_read` 读取受支持图片时可将图片直接提供给多模态模型 | | `file_search` / `content_search` | 文件名和内容搜索 | | `bash` | 在 workspace 中执行 Shell 命令 | | `http_request` / `web_fetch` | HTTP 请求和网页文本抽取 | diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 45b9018..ac2dfe6 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -201,6 +201,8 @@ Gateway 在 `/` 提供随二进制编译的 HTML/CSS/JavaScript，不依赖外 WebUI/TUI 文件字节通过受鉴权的 HTTP 接口流式传输，WebSocket 只携带短期 `upload_id` 和结构化附件描述。`UploadRegistry` 在内存中按 `cli_chat` chat scope 校验并消费待发送上传；消息继续以 `media_refs` 保存 Gateway 本地路径，不建立永久附件资产。下载接口必须通过 client、session、message 和附件序号反查路径，不能接受客户端路径。历史附件路径失效属于正常状态，不得影响历史文本读取。待发送但未进入消息的上传由 `TaskSupervisor` 所有的限时清理任务回收。Agent 上下文会为所有媒体注入内部路径清单，客户端响应不得暴露该路径。 +工具默认通过 `ToolResult` 返回文本；需要把图片等产物交给模型时，通过 `Tool::execute_with_media` 返回文本和结构化 `MediaRef`。工具只负责经过自身路径策略校验后声明媒体，不感知当前模型或 Provider。`AgentLoop` 仅将最新连续工具结果批次的媒体交给 `MediaHandlerRegistry`，旧工具媒体只回放文本和路径，避免历史 Base64 膨胀。OpenAI-compatible Provider 保持 `tool` 结果为文本，并在完整工具批次后构造仅存在于请求内的临时多模态 `user` 消息；Anthropic Provider 将媒体放入对应 `tool_result.content`。媒体加载、格式或能力检查失败必须降级成文本，不得使历史记录不可读取。 + `AuthManager` 默认保护所有管理 API 和 `/ws`。静态资源、`/health`、`/api/auth/status` 与 `/api/auth/pair` 保持公开，使未配对浏览器只能加载配对界面。`picobot pair` 使用权限为 `0600` 的本机管理密钥调用仅接受真实回环连接的 `/api/auth/code`；反向代理即使从回环连接也无法在没有该密钥时签发代码。配对码为 8 位、5 分钟有效、单次消费，并按来源实施失败锁定。浏览器收到 HttpOnly、SameSite=Strict Cookie，CLI 使用 Bearer token；服务端仅持久化 SHA-256 哈希。`--revoke-all` 的持久化成功后才清空内存令牌，活动 WebSocket 每 5 秒复核身份并回收已撤销连接。 同源 `/api/*` 管理接口只提供显式白名单能力： diff --git a/docs/FILE_TRANSFER_DESIGN.md b/docs/FILE_TRANSFER_DESIGN.md index 8cd5102..5307be5 100644 --- a/docs/FILE_TRANSFER_DESIGN.md +++ b/docs/FILE_TRANSFER_DESIGN.md @@ -259,21 +259,27 @@ Agent 继续使用 `send_message(files=...)`。文件路径由服务端内部产 ### 7.3 Agent 上下文中的路径 -构造发给模型的用户消息时，无论附件类型是否被模型原生支持，都先增加一个结构化附件清单。清单包含安全文件名、媒体类型和 Gateway 内部路径，例如： +构造发给模型的用户消息时，无论附件类型是否被模型原生支持，都把结构化附件清单和用户正文放在同一个文本内容块中。清单包含安全文件名、扩展名、媒体类型、MIME、当前可取得的大小、Gateway 内部路径和内容交付状态，例如： ```text -[附件清单：path 是 Gateway 内部存储路径，可供文件工具读取] +[随本条用户消息同时提交的附件。path 是 Gateway 内部存储路径，可供文件工具读取；content_delivery 说明附件内容是否另以模型原生内容块提供。] [ { "name": "report.pdf", + "extension": "pdf", "media_type": "file", - "path": "/gateway/media/report.pdf" + "mime_type": "application/pdf", + "size_bytes": 12345, + "path": "/gateway/media/report.pdf", + "content_delivery": "content is not embedded in this model request; path remains available to file tools" } ] ``` 随后再追加图片等原生多模态 content block。这样支持视觉输入的模型既能看到图片内容，也知道其文件路径；普通文件同样可以由 LLM 使用 `file_read`、Bash 等工具读取。附件路径只进入服务端到 Provider 的模型上下文，不进入 WebSocket/HTTP 客户端响应。 +`file_read` 读取 PNG、JPEG、GIF 或 WebP 时不把 Base64 当作工具文本返回，而是通过结构化工具媒体侧通道返回规范化路径。`AgentLoop` 根据当前模型能力构造原生图片块；只有最新连续工具结果批次携带图片内容，旧结果仅保留文本路径。OpenAI-compatible 请求把工具图片汇总为工具批次后的临时 `user` 多模态消息，Anthropic 请求把图片放入对应 `tool_result`。这些 Provider 请求视图不写入历史，消息仍只持久化路径引用。 + 历史路径已经失效时，清单仍反映消息所记录的原路径；工具读取失败应作为普通、可解释的“文件已移动或删除”结果返回，不能导致 Agent loop panic。 ## 8. WebUI 交互 diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs index a242ee4..57a020c 100644 --- a/src/agent/agent_loop.rs +++ b/src/agent/agent_loop.rs @@ -19,12 +19,31 @@ const MAX_TOOL_RESULT_CHARS: usize = 16_000; /// Minimum characters to keep when truncating const TRUNCATION_SUFFIX_LEN: usize = 200; +enum MediaOrigin<'a> { + User, + Tool(&'a str), + Message, +} + +fn should_include_message_media(messages: &[ChatMessage], index: usize) -> bool { + let message = &messages[index]; + if message.role != "tool" { + return true; + } + let active_tool_start = messages + .iter() + .rposition(|candidate| candidate.role != "tool") + .map_or(0, |last_non_tool| last_non_tool + 1); + index >= active_tool_start +} + /// Build content blocks from text and media, respecting model input capabilities fn build_content_blocks( text: &str, media_refs: &[MediaRef], input_types: &[String], registry: &MediaHandlerRegistry, + origin: MediaOrigin<'_>, ) -> Vec { let mut blocks = Vec::new(); @@ -56,20 +75,30 @@ fn build_content_blocks( "content_delivery": if native_input { "also included as a model-native content block" } else { - "content is not embedded; use a file tool with path to inspect it" + "content is not embedded in this model request; path remains available to file tools" }, }) }) .collect::>(); let manifest = serde_json::Value::Array(attachments).to_string(); + let note = match origin { + MediaOrigin::User if text.is_empty() => { + "用户发送了以下附件。path 是 Gateway 内部存储路径，可供文件工具读取；附件内容未必已嵌入模型输入。".to_string() + } + MediaOrigin::User => { + "随本条用户消息同时提交的附件。path 是 Gateway 内部存储路径，可供文件工具读取；content_delivery 说明附件内容是否另以模型原生内容块提供。".to_string() + } + MediaOrigin::Tool(tool_name) => format!( + "工具 {tool_name} 返回了以下附件。content_delivery 说明附件内容是否另以模型原生内容块提供。" + ), + MediaOrigin::Message => { + "本条消息包含以下附件。content_delivery 说明附件内容是否另以模型原生内容块提供。".to_string() + } + }; let message_text = if text.is_empty() { - format!( - "[用户发送了以下附件。path 是 Gateway 内部存储路径，可供文件工具读取；附件内容未必已嵌入模型输入。]\n{manifest}" - ) + format!("[{note}]\n{manifest}") } else { - format!( - "{text}\n\n[随本条用户消息同时提交的附件。path 是 Gateway 内部存储路径，可供文件工具读取；content_delivery 说明附件内容是否另以模型原生内容块提供。]\n{manifest}" - ) + format!("{text}\n\n[{note}]\n{manifest}") }; blocks.push(ContentBlock::text(message_text)); @@ -422,15 +451,21 @@ impl AgentLoop { &self.tools } - fn chat_message_to_llm_message(&self, m: &ChatMessage) -> Message { - let content = if m.media_refs.is_empty() { + fn chat_message_to_llm_message(&self, m: &ChatMessage, include_media: bool) -> Message { + let content = if m.media_refs.is_empty() || !include_media { vec![ContentBlock::text(&m.content)] } else { + let origin = match m.role.as_str() { + "user" => MediaOrigin::User, + "tool" => MediaOrigin::Tool(m.tool_name.as_deref().unwrap_or("unknown")), + _ => MediaOrigin::Message, + }; build_content_blocks( &m.content, &m.media_refs, &self.input_types, &self.media_registry, + origin, ) }; @@ -444,6 +479,17 @@ impl AgentLoop { } } + fn messages_for_llm(&self, messages: &[ChatMessage]) -> Vec { + messages + .iter() + .enumerate() + .map(|(index, message)| { + let include_media = should_include_message_media(messages, index); + self.chat_message_to_llm_message(message, include_media) + }) + .collect() + } + /// Process a message using the provided conversation history. /// History management is handled externally by SessionManager. /// @@ -501,10 +547,7 @@ impl AgentLoop { } // Convert messages to LLM format - let messages_for_llm: Vec = messages - .iter() - .map(|m| self.chat_message_to_llm_message(m)) - .collect(); + let messages_for_llm = self.messages_for_llm(&messages); // Build request let tools = if self.tools.has_tools() { @@ -601,19 +644,21 @@ impl AgentLoop { "Loop warning: {}", msg ); - let tool_message = ChatMessage::tool( + let tool_message = ChatMessage::tool_with_media( tool_call.id.clone(), tool_call.name.clone(), format!("{}\n\n[上一条结果]\n{}", msg, truncated_output), + result.media_refs.clone(), ); messages.push(tool_message.clone()); emitted_messages.push(tool_message); } LoopDetectionResult::Ok => { - let tool_message = ChatMessage::tool( + let tool_message = ChatMessage::tool_with_media( tool_call.id.clone(), tool_call.name.clone(), truncated_output, + result.media_refs.clone(), ); messages.push(tool_message.clone()); emitted_messages.push(tool_message); @@ -641,10 +686,7 @@ impl AgentLoop { messages.push(summary_request); // Convert messages to LLM format - let messages_for_llm: Vec = messages - .iter() - .map(|m| self.chat_message_to_llm_message(m)) - .collect(); + let messages_for_llm = self.messages_for_llm(&messages); let request = ChatCompletionRequest { messages: messages_for_llm, @@ -784,10 +826,14 @@ impl AgentLoop { } }; - match tool.execute(tool_call.arguments.clone()).await { - Ok(result) => { + match tool.execute_with_media(tool_call.arguments.clone()).await { + Ok(result_with_media) => { + let result = result_with_media.result; if result.success { - ToolExecutionOutcome::success(result.output) + ToolExecutionOutcome::success_with_media( + result.output, + result_with_media.media_refs, + ) } else { let error = result.error.unwrap_or_default(); ToolExecutionOutcome::failure(format!("Error: {}", error), Some(error)) @@ -805,6 +851,8 @@ impl AgentLoop { mod tests { use super::*; use crate::observability::{MultiObserver, Observer}; + use crate::providers::{ChatCompletionResponse, Usage}; + use crate::tools::FileReadTool; struct TestObserver { events: std::sync::Mutex>, @@ -844,6 +892,113 @@ mod tests { assert_eq!(multi.len(), 1); } + struct ToolMediaProvider { + image_path: String, + requests: std::sync::Mutex>, + } + + #[async_trait::async_trait] + impl LLMProvider for ToolMediaProvider { + async fn chat( + &self, + request: ChatCompletionRequest, + ) -> Result> { + let call_number = { + let mut requests = self.requests.lock().unwrap(); + requests.push(request); + requests.len() + }; + Ok(ChatCompletionResponse { + id: format!("response-{call_number}"), + model: "vision-test".to_string(), + content: if call_number == 1 { + String::new() + } else { + "image seen".to_string() + }, + reasoning_content: None, + tool_calls: if call_number == 1 { + vec![ToolCall { + id: "call-image".to_string(), + name: "file_read".to_string(), + arguments: serde_json::json!({ "path": self.image_path }), + }] + } else { + Vec::new() + }, + usage: Usage { + prompt_tokens: 1, + completion_tokens: 1, + total_tokens: 2, + cached_tokens: None, + cache_read_input_tokens: None, + cache_creation_input_tokens: None, + }, + }) + } + + fn ptype(&self) -> &str { + "test" + } + + fn name(&self) -> &str { + "tool-media-test" + } + + fn model_id(&self) -> &str { + "vision-test" + } + } + + #[tokio::test] + async fn file_read_media_reaches_the_next_model_iteration() { + use std::io::Write; + + let mut image = tempfile::Builder::new().suffix(".png").tempfile().unwrap(); + image.write_all(b"\x89PNG\r\n\x1a\nminimal").unwrap(); + let provider = Arc::new(ToolMediaProvider { + image_path: image.path().to_string_lossy().into_owned(), + requests: std::sync::Mutex::new(Vec::new()), + }); + let tools = Arc::new(ToolRegistry::new()); + tools.register(FileReadTool::new()); + let agent = AgentLoop::with_provider_and_tools( + provider.clone(), + tools, + 2, + "vision-test".to_string(), + std::env::current_dir().unwrap(), + vec!["text".to_string(), "image".to_string()], + ); + + let result = agent + .process(vec![ChatMessage::user("inspect the image")]) + .await + .unwrap(); + + assert_eq!(result.final_response.content, "image seen"); + let requests = provider.requests.lock().unwrap(); + assert_eq!(requests.len(), 2); + let tool_result = requests[1] + .messages + .iter() + .find(|message| message.role == "tool") + .unwrap(); + assert!( + tool_result + .content + .iter() + .any(|block| matches!(block, ContentBlock::ImageUrl { .. })) + ); + assert!(result.emitted_messages.iter().any(|message| { + message.role == "tool" + && message + .media_refs + .iter() + .any(|media| media.media_type == "image") + })); + } + #[test] fn test_should_execute_in_parallel_single_tool() { // Would need a proper setup with AgentLoop to test fully @@ -904,6 +1059,7 @@ mod tests { }], &[], ®istry, + MediaOrigin::User, ); assert_eq!(blocks.len(), 1); @@ -912,7 +1068,7 @@ mod tests { && text.contains("随本条用户消息同时提交的附件") && text.contains("missing.png") && text.contains("\"media_type\":\"image\"") - && text.contains("content is not embedded"))); + && text.contains("content is not embedded in this model request"))); } #[test] @@ -926,6 +1082,7 @@ mod tests { }], &[], ®istry, + MediaOrigin::User, ); assert_eq!(blocks.len(), 1); @@ -935,7 +1092,7 @@ mod tests { && text.contains("application/vnd.openxmlformats-officedocument.wordprocessingml.document") && text.contains("\"extension\":\"docx\"") && text.contains("\"size_bytes\":null") - && text.contains("content is not embedded"))); + && text.contains("content is not embedded in this model request"))); } #[test] @@ -943,7 +1100,7 @@ mod tests { use std::io::Write; let mut image = tempfile::Builder::new().suffix(".png").tempfile().unwrap(); - image.write_all(b"image bytes").unwrap(); + image.write_all(b"\x89PNG\r\n\x1a\nminimal").unwrap(); let path = image.path().to_string_lossy().into_owned(); let registry = MediaHandlerRegistry::with_defaults(); @@ -955,6 +1112,7 @@ mod tests { }], &["image".to_string()], ®istry, + MediaOrigin::User, ); assert!(matches!(blocks.first(), Some(ContentBlock::Text { text }) @@ -964,6 +1122,79 @@ mod tests { && text.contains("model-native content block"))); assert!(matches!(blocks.get(1), Some(ContentBlock::ImageUrl { .. }))); } + + #[test] + fn test_build_content_blocks_labels_tool_media_without_user_wording() { + use std::io::Write; + + let mut image = tempfile::Builder::new().suffix(".png").tempfile().unwrap(); + image.write_all(b"\x89PNG\r\n\x1a\nminimal").unwrap(); + let path = image.path().to_string_lossy().into_owned(); + let registry = MediaHandlerRegistry::with_defaults(); + + let blocks = build_content_blocks( + "Image file ready for visual inspection.", + &[MediaRef { + path, + media_type: "image".to_string(), + }], + &["image".to_string()], + ®istry, + MediaOrigin::Tool("file_read"), + ); + + assert!(matches!(blocks.first(), Some(ContentBlock::Text { text }) + if text.contains("工具 file_read 返回了以下附件") + && !text.contains("用户消息"))); + assert!(matches!(blocks.get(1), Some(ContentBlock::ImageUrl { .. }))); + } + + #[test] + fn only_the_trailing_tool_batch_replays_tool_media() { + let mut messages = vec![ + ChatMessage::assistant_with_tool_calls( + "", + vec![ToolCall { + id: "old-call".to_string(), + name: "file_read".to_string(), + arguments: serde_json::json!({}), + }], + ), + ChatMessage::tool_with_media( + "old-call", + "file_read", + "old", + vec![MediaRef { + path: "/tmp/old.png".to_string(), + media_type: "image".to_string(), + }], + ), + ChatMessage::assistant("continue"), + ChatMessage::assistant_with_tool_calls( + "", + vec![ToolCall { + id: "new-call".to_string(), + name: "file_read".to_string(), + arguments: serde_json::json!({}), + }], + ), + ChatMessage::tool_with_media( + "new-call", + "file_read", + "new", + vec![MediaRef { + path: "/tmp/new.png".to_string(), + media_type: "image".to_string(), + }], + ), + ]; + + assert!(!should_include_message_media(&messages, 1)); + assert!(should_include_message_media(&messages, 4)); + + messages.push(ChatMessage::user("next turn")); + assert!(!should_include_message_media(&messages, 4)); + } } #[derive(Debug)] diff --git a/src/agent/media_handler.rs b/src/agent/media_handler.rs index 5765bb0..6569492 100644 --- a/src/agent/media_handler.rs +++ b/src/agent/media_handler.rs @@ -4,6 +4,8 @@ use std::io::Read; use crate::bus::message::ContentBlock; +const MAX_NATIVE_IMAGE_BYTES: u64 = 10 * 1024 * 1024; + pub trait MediaHandler: Send + Sync { fn media_type(&self) -> &str; fn handle(&self, path: &str) -> Result, MediaHandlerError>; @@ -41,28 +43,60 @@ impl MediaHandler for ImageHandler { } fn handle(&self, path: &str) -> Result, MediaHandlerError> { - let (mime_type, base64_data) = - encode_image_to_base64(path).map_err(MediaHandlerError::Io)?; + let (mime_type, base64_data) = encode_image_to_base64(path)?; let url = format!("data:{};base64,{}", mime_type, base64_data); Ok(vec![ContentBlock::image_url(url)]) } } -fn encode_image_to_base64(path: &str) -> Result<(String, String), std::io::Error> { +fn encode_image_to_base64(path: &str) -> Result<(String, String), MediaHandlerError> { use base64::{Engine as _, engine::general_purpose::STANDARD}; - let mut file = std::fs::File::open(path)?; - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; + let metadata = std::fs::metadata(path).map_err(MediaHandlerError::Io)?; + if metadata.len() > MAX_NATIVE_IMAGE_BYTES { + return Err(MediaHandlerError::UnsupportedFormat(format!( + "image is too large: {} bytes (max {} bytes)", + metadata.len(), + MAX_NATIVE_IMAGE_BYTES + ))); + } let mime = mime_guess::from_path(path) .first_or_octet_stream() .to_string(); + if !matches!( + mime.as_str(), + "image/png" | "image/jpeg" | "image/gif" | "image/webp" + ) { + return Err(MediaHandlerError::UnsupportedFormat(format!( + "unsupported image MIME type: {mime}" + ))); + } + + let mut file = std::fs::File::open(path).map_err(MediaHandlerError::Io)?; + let mut buffer = Vec::new(); + file.read_to_end(&mut buffer) + .map_err(MediaHandlerError::Io)?; + if !valid_image_signature(&buffer, &mime) { + return Err(MediaHandlerError::UnsupportedFormat(format!( + "file content does not match image MIME type: {mime}" + ))); + } let encoded = STANDARD.encode(&buffer); Ok((mime, encoded)) } +fn valid_image_signature(bytes: &[u8], mime: &str) -> bool { + match mime { + "image/png" => bytes.starts_with(b"\x89PNG\r\n\x1a\n"), + "image/jpeg" => bytes.starts_with(&[0xff, 0xd8, 0xff]), + "image/gif" => bytes.starts_with(b"GIF87a") || bytes.starts_with(b"GIF89a"), + "image/webp" => bytes.len() >= 12 && bytes.starts_with(b"RIFF") && &bytes[8..12] == b"WEBP", + _ => false, + } +} + pub struct MediaHandlerRegistry { handlers: HashMap>, } diff --git a/src/bus/message.rs b/src/bus/message.rs index 9d8dd57..56ae5ef 100644 --- a/src/bus/message.rs +++ b/src/bus/message.rs @@ -215,13 +215,22 @@ impl ChatMessage { tool_call_id: impl Into, tool_name: impl Into, content: impl Into, + ) -> Self { + Self::tool_with_media(tool_call_id, tool_name, content, Vec::new()) + } + + pub fn tool_with_media( + tool_call_id: impl Into, + tool_name: impl Into, + content: impl Into, + media_refs: Vec, ) -> Self { Self { id: uuid::Uuid::new_v4().to_string(), role: "tool".to_string(), content: content.into(), reasoning_content: None, - media_refs: Vec::new(), + media_refs, timestamp: current_timestamp(), tool_call_id: Some(tool_call_id.into()), tool_name: Some(tool_name.into()), diff --git a/src/observability/mod.rs b/src/observability/mod.rs index 6d81ae3..ee756f5 100644 --- a/src/observability/mod.rs +++ b/src/observability/mod.rs @@ -5,6 +5,8 @@ use std::time::Duration; +use crate::bus::MediaRef; + /// Events emitted during agent and tool execution. #[derive(Debug, Clone)] pub enum ObserverEvent { @@ -57,6 +59,8 @@ pub struct ToolExecutionOutcome { pub error_reason: Option, /// How long the tool took to execute. pub duration: Duration, + /// Structured media returned by the tool for the next model iteration. + pub media_refs: Vec, } impl ToolExecutionOutcome { @@ -67,6 +71,18 @@ impl ToolExecutionOutcome { success: true, error_reason: None, duration: Duration::ZERO, + media_refs: Vec::new(), + } + } + + /// Create a successful outcome carrying structured media artifacts. + pub fn success_with_media(output: String, media_refs: Vec) -> Self { + Self { + output, + success: true, + error_reason: None, + duration: Duration::ZERO, + media_refs, } } @@ -77,6 +93,7 @@ impl ToolExecutionOutcome { success: false, error_reason, duration: Duration::ZERO, + media_refs: Vec::new(), } } } diff --git a/src/providers/anthropic.rs b/src/providers/anthropic.rs index f925dea..975d68d 100644 --- a/src/providers/anthropic.rs +++ b/src/providers/anthropic.rs @@ -5,7 +5,7 @@ use std::collections::HashMap; use std::time::Duration; use super::traits::Usage; -use super::{ChatCompletionRequest, ChatCompletionResponse, LLMProvider, Tool, ToolCall}; +use super::{ChatCompletionRequest, ChatCompletionResponse, LLMProvider, Message, Tool, ToolCall}; use crate::bus::message::ContentBlock; use crate::storage::Storage; use std::sync::Arc; @@ -139,6 +139,44 @@ struct AnthropicMessage { content: Vec, } +fn convert_messages(messages: &[Message]) -> Vec { + messages + .iter() + .map(|message| { + let role = if message.role == "tool" { + "user".to_string() + } else { + message.role.clone() + }; + let content = if let Some(ref tool_call_id) = message.tool_call_id { + vec![serde_json::json!({ + "type": "tool_result", + "tool_use_id": tool_call_id, + "content": convert_content_blocks(&message.content, false), + })] + } else { + let mut blocks = convert_content_blocks(&message.content, message.role == "system"); + if let Some(tool_calls) = message + .tool_calls + .as_ref() + .filter(|calls| !calls.is_empty()) + { + for tool_call in tool_calls { + blocks.push(serde_json::json!({ + "type": "tool_use", + "id": tool_call.id, + "name": tool_call.name, + "input": tool_call.arguments, + })); + } + } + blocks + }; + AnthropicMessage { role, content } + }) + .collect() +} + #[derive(Serialize)] struct AnthropicTool { name: String, @@ -216,50 +254,7 @@ impl LLMProvider for AnthropicProvider { let body = AnthropicRequest { model: self.model_id.clone(), - messages: request - .messages - .iter() - .map(|m| { - let role = if m.role == "tool" { - // Anthropic uses "user" role for tool result messages - "user".to_string() - } else { - m.role.clone() - }; - let content = if let Some(ref tc_id) = m.tool_call_id { - // Tool result: wrap as tool_result content block - let output = m - .content - .iter() - .filter_map(|b| match b { - ContentBlock::Text { text } => Some(text.as_str()), - _ => None, - }) - .collect::>() - .join(""); - vec![serde_json::json!({ - "type": "tool_result", - "tool_use_id": tc_id, - "content": output, - })] - } else { - let mut blocks = convert_content_blocks(&m.content, m.role == "system"); - // Append tool_use blocks from assistant messages with tool calls - if let Some(tool_calls) = m.tool_calls.as_ref().filter(|c| !c.is_empty()) { - for tc in tool_calls { - blocks.push(serde_json::json!({ - "type": "tool_use", - "id": tc.id, - "name": tc.name, - "input": tc.arguments, - })); - } - } - blocks - }; - AnthropicMessage { role, content } - }) - .collect(), + messages: convert_messages(&request.messages), max_tokens, temperature: request.temperature.or(self.temperature), tools, @@ -482,4 +477,29 @@ mod tests { let value = serde_json::to_value(tool).unwrap(); assert_eq!(value["cache_control"]["type"], "ephemeral"); } + + #[test] + fn tool_result_preserves_native_image_blocks() { + let messages = vec![Message { + role: "tool".to_string(), + content: vec![ + ContentBlock::text("image ready"), + ContentBlock::image_url("data:image/png;base64,AAAA"), + ], + reasoning_content: None, + tool_call_id: Some("call_1".to_string()), + name: Some("file_read".to_string()), + tool_calls: None, + }]; + + let converted = convert_messages(&messages); + let result = &converted[0].content[0]; + + assert_eq!(converted[0].role, "user"); + assert_eq!(result["type"], "tool_result"); + assert_eq!(result["content"][0]["type"], "text"); + assert_eq!(result["content"][1]["type"], "image"); + assert_eq!(result["content"][1]["source"]["media_type"], "image/png"); + assert_eq!(result["content"][1]["source"]["data"], "AAAA"); + } } diff --git a/src/providers/openai.rs b/src/providers/openai.rs index 918c16b..ceab97a 100644 --- a/src/providers/openai.rs +++ b/src/providers/openai.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; use std::time::Duration; use super::traits::Usage; -use super::{ChatCompletionRequest, ChatCompletionResponse, LLMProvider, ToolCall}; +use super::{ChatCompletionRequest, ChatCompletionResponse, LLMProvider, Message, ToolCall}; use crate::bus::message::ContentBlock; use crate::storage::Storage; use std::sync::Arc; @@ -32,6 +32,116 @@ fn convert_content_blocks(blocks: &[ContentBlock]) -> Value { ) } +fn text_content(blocks: &[ContentBlock]) -> String { + blocks + .iter() + .filter_map(|block| match block { + ContentBlock::Text { text } => Some(text.as_str()), + ContentBlock::ImageUrl { .. } => None, + }) + .collect::>() + .join("\n") +} + +fn regular_message_json(message: &Message) -> Value { + if message.role == "tool" { + json!({ + "role": message.role, + "content": text_content(&message.content), + "tool_call_id": message.tool_call_id, + "name": message.name, + }) + } else if message.role == "assistant" + && message + .tool_calls + .as_ref() + .is_some_and(|calls| !calls.is_empty()) + { + let mut value = json!({ + "role": message.role, + "content": convert_content_blocks(&message.content), + "tool_calls": message.tool_calls.as_ref().map(|calls| { + calls.iter().map(|call| json!({ + "id": call.id, + "type": "function", + "function": { + "name": call.name, + "arguments": serde_json::to_string(&call.arguments).unwrap_or_else(|_| "null".to_string()) + } + })).collect::>() + }) + }); + if let Some(ref reasoning_content) = message.reasoning_content { + value["reasoning_content"] = json!(reasoning_content); + } + value + } else { + let mut value = json!({ + "role": message.role, + "content": convert_content_blocks(&message.content) + }); + if message.role == "assistant" + && let Some(ref reasoning_content) = message.reasoning_content + { + value["reasoning_content"] = json!(reasoning_content); + } + value + } +} + +/// OpenAI-compatible APIs do not consistently accept image parts on `tool` +/// messages. Keep the tool-call contract textual and append one temporary user +/// message containing media returned by the complete contiguous tool batch. +fn convert_messages(messages: &[Message]) -> Vec { + let mut converted = Vec::with_capacity(messages.len()); + let mut tool_media = Vec::new(); + let mut tool_media_labels = Vec::new(); + + let flush_tool_media = + |converted: &mut Vec, tool_media: &mut Vec, labels: &mut Vec| { + if tool_media.is_empty() { + return; + } + let mut content = vec![json!({ + "type": "text", + "text": format!("[以下媒体由本轮工具调用返回]\n{}", labels.join("\n")), + })]; + content.append(tool_media); + converted.push(json!({ + "role": "user", + "content": content, + })); + labels.clear(); + }; + + for message in messages { + if message.role != "tool" { + flush_tool_media(&mut converted, &mut tool_media, &mut tool_media_labels); + } + converted.push(regular_message_json(message)); + if message.role == "tool" { + for block in &message.content { + if let ContentBlock::ImageUrl { image_url } = block { + let label = format!( + "- {} / {}", + message.name.as_deref().unwrap_or("tool"), + message.tool_call_id.as_deref().unwrap_or("unknown") + ); + if tool_media_labels.last() != Some(&label) { + tool_media_labels.push(label); + } + tool_media.push(json!({ + "type": "image_url", + "image_url": { "url": image_url.url }, + })); + } + } + } + } + flush_tool_media(&mut converted, &mut tool_media, &mut tool_media_labels); + converted +} + pub struct OpenAIProvider { client: Client, name: String, @@ -83,46 +193,7 @@ impl OpenAIProvider { fn build_request_body(&self, request: &ChatCompletionRequest) -> Value { let mut body = json!({ "model": self.model_id, - "messages": request.messages.iter().map(|m| { - if m.role == "tool" { - json!({ - "role": m.role, - "content": convert_content_blocks(&m.content), - "tool_call_id": m.tool_call_id, - "name": m.name, - }) - } else if m.role == "assistant" && m.tool_calls.as_ref().is_some_and(|c| !c.is_empty()) { - let mut msg = json!({ - "role": m.role, - "content": convert_content_blocks(&m.content), - "tool_calls": m.tool_calls.as_ref().map(|calls| { - calls.iter().map(|call| json!({ - "id": call.id, - "type": "function", - "function": { - "name": call.name, - "arguments": serde_json::to_string(&call.arguments).unwrap_or_else(|_| "null".to_string()) - } - })).collect::>() - }) - }); - if let Some(ref rc) = m.reasoning_content { - msg["reasoning_content"] = json!(rc); - } - msg - } else { - let mut msg = json!({ - "role": m.role, - "content": convert_content_blocks(&m.content) - }); - if m.role == "assistant" - && let Some(ref rc) = m.reasoning_content - { - msg["reasoning_content"] = json!(rc); - } - msg - } - }).collect::>(), + "messages": convert_messages(&request.messages), "temperature": request.temperature.or(self.temperature).unwrap_or(0.7), "max_tokens": request.max_tokens.or(self.max_tokens), }); @@ -450,6 +521,37 @@ mod tests { ); } + #[test] + fn tool_images_are_appended_after_the_complete_tool_batch() { + let messages = vec![ + Message::tool("call_1", "file_read", "first image"), + Message { + role: "tool".to_string(), + content: vec![ + ContentBlock::text("second image"), + ContentBlock::image_url("data:image/png;base64,AAAA"), + ], + reasoning_content: None, + tool_call_id: Some("call_2".to_string()), + name: Some("file_read".to_string()), + tool_calls: None, + }, + ]; + + let converted = convert_messages(&messages); + + assert_eq!(converted.len(), 3); + assert_eq!(converted[0]["role"], "tool"); + assert_eq!(converted[1]["role"], "tool"); + assert_eq!(converted[2]["role"], "user"); + assert_eq!(converted[2]["content"][1]["type"], "image_url"); + assert_eq!( + converted[2]["content"][1]["image_url"]["url"], + "data:image/png;base64,AAAA" + ); + assert_eq!(converted[1]["content"], "second image"); + } + #[test] fn test_decode_response_accepts_null_tool_calls() { let text = r#"{ diff --git a/src/tools/file_read.rs b/src/tools/file_read.rs index b0e5bf3..825884c 100644 --- a/src/tools/file_read.rs +++ b/src/tools/file_read.rs @@ -1,9 +1,11 @@ use async_trait::async_trait; use encoding_rs::*; use serde_json::json; +use std::io::Read; +use crate::bus::MediaRef; use crate::tools::path_utils; -use crate::tools::traits::{Tool, ToolResult}; +use crate::tools::traits::{Tool, ToolResult, ToolResultWithMedia}; const MAX_CHARS: usize = 128_000; const MAX_FILE_BYTES: u64 = 5 * 1024 * 1024; @@ -39,7 +41,7 @@ impl Tool for FileReadTool { } fn description(&self) -> &str { - "Read the contents of a file. Returns numbered lines. Use offset and limit to paginate through large files." + "Read a file. Text returns numbered lines with offset/limit pagination; supported images are provided as visual media to vision-capable models." } fn parameters_schema(&self) -> serde_json::Value { @@ -266,6 +268,111 @@ impl Tool for FileReadTool { } } } + + async fn execute_with_media( + &self, + args: serde_json::Value, + ) -> anyhow::Result { + if let Some(image_result) = self.inspect_image_result(&args) { + return Ok(image_result); + } + self.execute(args).await.map(Into::into) + } +} + +impl FileReadTool { + fn inspect_image_result(&self, args: &serde_json::Value) -> Option { + let path = args.get("path")?.as_str()?; + let resolved = path_utils::resolve_path(path, self.allowed_dir.as_deref()).ok()?; + let mime = mime_guess::from_path(&resolved) + .first_or_octet_stream() + .to_string(); + if !is_supported_image_mime(&mime) { + return None; + } + + let failure = |error: String| ToolResultWithMedia { + result: ToolResult { + success: false, + output: String::new(), + error: Some(error), + }, + media_refs: Vec::new(), + }; + + if !resolved.exists() { + return Some(failure(format!("File not found: {path}"))); + } + if !resolved.is_file() { + return Some(failure(format!("Not a file: {path}"))); + } + let metadata = match std::fs::metadata(&resolved) { + Ok(metadata) => metadata, + Err(error) => { + return Some(failure(format!("Failed to inspect file: {error}"))); + } + }; + if metadata.len() > MAX_FILE_BYTES { + return Some(failure(format!( + "File too large to read safely: {} bytes (max {} bytes).", + metadata.len(), + MAX_FILE_BYTES + ))); + } + if let Err(error) = validate_image_signature(&resolved, &mime) { + return Some(failure(error)); + } + + let canonical = std::fs::canonicalize(&resolved).unwrap_or(resolved); + let canonical_path = canonical.to_string_lossy().into_owned(); + Some(ToolResultWithMedia { + result: ToolResult { + success: true, + output: format!( + "Image file ready for visual inspection.\nPath: {canonical_path}\nMIME: {mime}\nSize: {} bytes", + metadata.len() + ), + error: None, + }, + media_refs: vec![MediaRef { + path: canonical_path, + media_type: "image".to_string(), + }], + }) + } +} + +fn is_supported_image_mime(mime: &str) -> bool { + matches!( + mime, + "image/png" | "image/jpeg" | "image/gif" | "image/webp" + ) +} + +fn validate_image_signature(path: &std::path::Path, mime: &str) -> Result<(), String> { + let mut file = + std::fs::File::open(path).map_err(|error| format!("Failed to open image file: {error}"))?; + let mut header = [0_u8; 12]; + let read = file + .read(&mut header) + .map_err(|error| format!("Failed to inspect image file: {error}"))?; + let header = &header[..read]; + let valid = match mime { + "image/png" => header.starts_with(b"\x89PNG\r\n\x1a\n"), + "image/jpeg" => header.starts_with(&[0xff, 0xd8, 0xff]), + "image/gif" => header.starts_with(b"GIF87a") || header.starts_with(b"GIF89a"), + "image/webp" => { + header.len() >= 12 && header.starts_with(b"RIFF") && &header[8..12] == b"WEBP" + } + _ => false, + }; + if valid { + Ok(()) + } else { + Err(format!( + "File content does not match the expected image format: {mime}" + )) + } } fn decode_text(bytes: &[u8]) -> (Option, Option<&'static str>) { @@ -414,4 +521,48 @@ mod tests { assert!(!result.success); assert!(result.error.unwrap().contains("Binary file too large")); } + + #[tokio::test] + async fn image_read_returns_structured_media_without_base64_text() { + let mut file = tempfile::Builder::new().suffix(".png").tempfile().unwrap(); + file.write_all(b"\x89PNG\r\n\x1a\nminimal").unwrap(); + + let result = FileReadTool::new() + .execute_with_media(json!({ "path": file.path() })) + .await + .unwrap(); + + assert!(result.result.success); + assert_eq!(result.media_refs.len(), 1); + assert_eq!(result.media_refs[0].media_type, "image"); + assert_eq!( + result.media_refs[0].path, + std::fs::canonicalize(file.path()) + .unwrap() + .to_string_lossy() + ); + assert!(result.result.output.contains("MIME: image/png")); + assert!(!result.result.output.contains("base64")); + } + + #[tokio::test] + async fn image_read_rejects_mismatched_file_signature() { + let mut file = tempfile::Builder::new().suffix(".png").tempfile().unwrap(); + file.write_all(b"not a png").unwrap(); + + let result = FileReadTool::new() + .execute_with_media(json!({ "path": file.path() })) + .await + .unwrap(); + + assert!(!result.result.success); + assert!(result.media_refs.is_empty()); + assert!( + result + .result + .error + .unwrap() + .contains("does not match the expected image format") + ); + } } diff --git a/src/tools/mod.rs b/src/tools/mod.rs index e9a2f9d..9ddb076 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -41,7 +41,7 @@ pub use pty::{PtyManager, PtyTool}; pub use registry::ToolRegistry; pub use send_message::SendMessageTool; pub use todo::TodoTool; -pub use traits::{OutboundMessenger, Tool, ToolResult}; +pub use traits::{OutboundMessenger, Tool, ToolResult, ToolResultWithMedia}; pub use web_fetch::WebFetchTool; use crate::agent::SubAgentManager; diff --git a/src/tools/traits.rs b/src/tools/traits.rs index b2213a5..560dd08 100644 --- a/src/tools/traits.rs +++ b/src/tools/traits.rs @@ -1,4 +1,4 @@ -use crate::bus::{MediaItem, MessageSource}; +use crate::bus::{MediaItem, MediaRef, MessageSource}; use async_trait::async_trait; #[derive(Debug, Clone)] @@ -8,6 +8,23 @@ pub struct ToolResult { pub error: Option, } +/// A tool result plus media artifacts that should be made available to a +/// capable model on the next agent iteration. +#[derive(Debug, Clone)] +pub struct ToolResultWithMedia { + pub result: ToolResult, + pub media_refs: Vec, +} + +impl From for ToolResultWithMedia { + fn from(result: ToolResult) -> Self { + Self { + result, + media_refs: Vec::new(), + } + } +} + #[async_trait] pub trait Tool: Send + Sync + 'static { fn name(&self) -> &str; @@ -15,6 +32,15 @@ pub trait Tool: Send + Sync + 'static { fn parameters_schema(&self) -> serde_json::Value; async fn execute(&self, args: serde_json::Value) -> anyhow::Result; + /// Execute the tool and return structured media artifacts when applicable. + /// Most tools only return text and use this default implementation. + async fn execute_with_media( + &self, + args: serde_json::Value, + ) -> anyhow::Result { + self.execute(args).await.map(Into::into) + } + /// Whether this tool is side-effect free and safe to parallelize. fn read_only(&self) -> bool { false