From ad9ff3bea84cdf061a9a851598d1bb888df9e46f Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Tue, 25 Aug 2026 10:54:35 -0600 Subject: [PATCH] style: revised a few stylistic choices after I changed my mind --- src/config/macro_policy.rs | 5 +++-- src/function/mod.rs | 36 ++++++++++++++++++++++++++---------- src/mcp/mod.rs | 1 + src/mcp/render.rs | 19 +++++++++++++------ src/repl/completer.rs | 1 + 5 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/config/macro_policy.rs b/src/config/macro_policy.rs index 835a5bb..f9b97cc 100644 --- a/src/config/macro_policy.rs +++ b/src/config/macro_policy.rs @@ -287,6 +287,7 @@ fn discover_macros_in(dirs: &[(MacroSource, PathBuf)]) -> Vec { #[cfg(test)] mod tests { use super::*; + use crate::repl; use crate::utils::get_env_name; use serial_test::serial; use std::path::Path; @@ -573,7 +574,7 @@ mod tests { None, None, None, - &crate::repl::builtin_command_names(), + &repl::builtin_command_names(), ); assert_eq!(state_of(&policy, "prompt"), &MacroState::ShadowedBuiltin); @@ -587,7 +588,7 @@ mod tests { None, None, None, - &crate::repl::builtin_command_names(), + &repl::builtin_command_names(), ); assert_eq!(state_of(&policy, "temp-role"), &MacroState::ShadowedBuiltin); diff --git a/src/function/mod.rs b/src/function/mod.rs index ce09734..d32a24c 100644 --- a/src/function/mod.rs +++ b/src/function/mod.rs @@ -787,9 +787,9 @@ impl Functions { name: invoke_function_name.clone(), description: formatdoc!( r#" - Invoke the specified tool on the {server} MCP server. Always call {describe_function_name} first to - find the correct invocation schema for the given tool. - "# + Invoke the specified tool on the {server} MCP server. Always call {describe_function_name} first to + find the correct invocation schema for the given tool. + "# ), parameters: JsonSchema { type_value: Some("object".to_string()), @@ -1963,6 +1963,7 @@ fn render_resource_content( "total_bytes": rendered.total_bytes, "next_offset": rendered.next_offset, }); + if let Some(next_offset) = rendered.next_offset { value["note"] = json!(format!( "Content truncated; re-call with offset={next_offset} to continue (max_bytes is \ @@ -1970,6 +1971,7 @@ fn render_resource_content( render::TEXT_MAX_BYTES_CLAMP )); } + Ok(value) } @@ -2010,6 +2012,7 @@ fn render_tool_result(mut result: Value, server: &str) -> Result { }), ); } + Ok(result) } @@ -2049,6 +2052,7 @@ fn render_tool_content_item(item: &mut Value, server: &str) -> Result<()> { } _ => {} } + Ok(()) } @@ -2068,6 +2072,7 @@ fn render_tool_blob( // One undecodable item must not sink the rest of the result. Err(error) => json!({ "error": format!("Failed to render blob content: {error}") }), }; + if let Some(map) = value.as_object_mut() { if let Some(mime_type) = mime_type && !map.contains_key("mime_type") @@ -2078,6 +2083,7 @@ fn render_tool_blob( map.insert("uri".to_string(), json!(uri)); } } + Ok(value) } @@ -2085,9 +2091,11 @@ fn clamp_tool_text(container: &mut Value) { let Some(text) = container.get("text").and_then(Value::as_str) else { return; }; + if text.len() <= render::TEXT_MAX_BYTES_CLAMP { return; } + let total_bytes = text.len(); let clamped = render::truncate_utf8(text, render::TEXT_MAX_BYTES_CLAMP).to_string(); let Some(map) = container.as_object_mut() else { @@ -2110,9 +2118,11 @@ fn clamp_metadata_field(object: &mut Value, key: &str) { let Some(text) = object.get(key).and_then(Value::as_str) else { return; }; + if text.len() <= render::METADATA_MAX_BYTES { return; } + let clamped = render::clamp_metadata(text); object[key] = json!(clamped); } @@ -2471,6 +2481,7 @@ mod tests { use rmcp::model::{CallToolResult, ContentBlock}; use serde_json::json; use serial_test::serial; + use std::process; use std::sync::Arc; fn call(name: &str, id: Option<&str>) -> ToolCall { @@ -2779,11 +2790,8 @@ mod tests { assert_eq!(MCP_INVOKE_META_FUNCTION_NAME_PREFIX, "mcp_invoke"); assert_eq!(MCP_SEARCH_META_FUNCTION_NAME_PREFIX, "mcp_search"); assert_eq!(MCP_DESCRIBE_META_FUNCTION_NAME_PREFIX, "mcp_describe"); - assert_eq!(crate::mcp::MCP_READ_META_FUNCTION_NAME_PREFIX, "mcp_read"); - assert_eq!( - crate::mcp::MCP_PROMPT_META_FUNCTION_NAME_PREFIX, - "mcp_prompt" - ); + assert_eq!(MCP_READ_META_FUNCTION_NAME_PREFIX, "mcp_read"); + assert_eq!(MCP_PROMPT_META_FUNCTION_NAME_PREFIX, "mcp_prompt"); } #[test] @@ -2866,7 +2874,9 @@ mod tests { #[test] fn functions_append_mcp_meta_creates_three_per_server() { let mut f = Functions::default(); + f.append_mcp_meta_functions(vec![tools_only("github")]); + assert_eq!(f.declarations().len(), 3); assert!(f.contains("mcp_invoke_github")); assert!(f.contains("mcp_search_github")); @@ -2876,7 +2886,9 @@ mod tests { #[test] fn functions_append_mcp_meta_multiple_servers() { let mut f = Functions::default(); + f.append_mcp_meta_functions(vec![tools_only("github"), tools_only("slack")]); + assert_eq!(f.declarations().len(), 6); assert!(f.contains("mcp_invoke_github")); assert!(f.contains("mcp_invoke_slack")); @@ -2892,7 +2904,9 @@ mod tests { #[test] fn functions_append_mcp_meta_resources_only_omits_invoke() { let mut f = Functions::default(); + f.append_mcp_meta_functions(vec![mcp_features("res", false, true, false)]); + assert_eq!(f.declarations().len(), 3); assert!(!f.contains("mcp_invoke_res")); assert!(!f.contains("mcp_prompt_res")); @@ -2904,7 +2918,9 @@ mod tests { #[test] fn functions_append_mcp_meta_all_capabilities_emits_five() { let mut f = Functions::default(); + f.append_mcp_meta_functions(vec![mcp_features("srv", true, true, true)]); + assert_eq!(f.declarations().len(), 5); assert!(f.contains("mcp_invoke_srv")); assert!(f.contains("mcp_search_srv")); @@ -3485,7 +3501,7 @@ mod tests { static COUNTER: AtomicU64 = AtomicU64::new(0); let cache_dir = env::temp_dir().join(format!( "coyote-read-blob-{}-{}", - std::process::id(), + process::id(), COUNTER.fetch_add(1, Ordering::Relaxed) )); fs::create_dir_all(&cache_dir).unwrap(); @@ -3623,7 +3639,7 @@ mod tests { static COUNTER: AtomicU64 = AtomicU64::new(0); let cache_dir = env::temp_dir().join(format!( "coyote-tool-blob-{}-{}", - std::process::id(), + process::id(), COUNTER.fetch_add(1, Ordering::Relaxed) )); fs::create_dir_all(&cache_dir).unwrap(); diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index afd52c5..99d8272 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -456,6 +456,7 @@ impl McpRegistry { ) }) .collect(); + features.sort_by(|a, b| a.name.cmp(&b.name)); features } diff --git a/src/mcp/render.rs b/src/mcp/render.rs index 37a719a..7ad7ba4 100644 --- a/src/mcp/render.rs +++ b/src/mcp/render.rs @@ -7,7 +7,7 @@ use base64::read::DecoderReader; use fancy_regex::Regex; use serde::Serialize; use sha2::{Digest, Sha256}; -use std::fmt; +use std::error::Error; use std::fs::{self, OpenOptions}; use std::io::{ErrorKind, Read, Write}; #[cfg(unix)] @@ -15,6 +15,7 @@ use std::io::{ErrorKind, Read, Write}; use std::os::unix::fs::OpenOptionsExt; use std::path::{Path, PathBuf}; use std::time::SystemTime; +use std::{fmt, io}; /// Default page size when the caller does not specify `max_bytes`. pub const DEFAULT_TEXT_MAX_BYTES: usize = 51_200; @@ -49,7 +50,7 @@ pub enum RenderError { InvalidPattern { pattern: String, error: String }, DecodedSizeExceeded, InvalidBase64(String), - Io(std::io::Error), + Io(io::Error), } impl fmt::Display for RenderError { @@ -71,8 +72,8 @@ impl fmt::Display for RenderError { } } -impl std::error::Error for RenderError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { +impl Error for RenderError { + fn source(&self) -> Option<&(dyn Error + 'static)> { match self { Self::Io(error) => Some(error), _ => None, @@ -80,8 +81,8 @@ impl std::error::Error for RenderError { } } -impl From for RenderError { - fn from(error: std::io::Error) -> Self { +impl From for RenderError { + fn from(error: io::Error) -> Self { Self::Io(error) } } @@ -145,6 +146,7 @@ pub fn render_text( end += 1; } } + let truncated = end < total_bytes; Ok(RenderedText { text: stream[start..end].to_string(), @@ -222,6 +224,7 @@ pub fn clamp_metadata(text: &str) -> String { if text.len() <= METADATA_MAX_BYTES { return text.to_string(); } + let clamped = truncate_utf8(text, METADATA_MAX_BYTES); format!("{clamped} [truncated: exceeds METADATA_MAX_BYTES ({METADATA_MAX_BYTES} bytes)]") } @@ -259,6 +262,7 @@ fn filter_lines(text: &str, pattern: &str) -> Result { out.push(format!("{}{marker}{line}", i + 1)); prev_kept = Some(i); } + Ok(out.join("\n")) } @@ -309,6 +313,7 @@ fn extension_for_mime(mime: Option<&str>) -> &'static str { && ext .bytes() .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit()); + if safe { ext } else { "bin" } } @@ -354,9 +359,11 @@ fn evict_oldest(mut entries: Vec, max_total: u64, protect: &Path) { if total <= max_total { break; } + if entry.path == *protect { continue; } + match fs::remove_file(&entry.path) { Ok(()) => total -= entry.size, Err(error) if error.kind() == ErrorKind::NotFound => total -= entry.size, diff --git a/src/repl/completer.rs b/src/repl/completer.rs index ee1807e..be439fd 100644 --- a/src/repl/completer.rs +++ b/src/repl/completer.rs @@ -224,6 +224,7 @@ fn list_prompts_blocking( Some(handle) => tokio::task::block_in_place(|| handle.block_on(fut)), None => tokio::runtime::Runtime::new().ok()?.block_on(fut), }; + result.ok()?.ok() }