From 8343c4a16bf9269ed29a98480fdc2b1d27f71f36 Mon Sep 17 00:00:00 2001 From: Jasper Date: Thu, 20 Aug 2026 14:03:51 +0000 Subject: [PATCH] fix: decouple source file and tool response limits (#11391) --- .../src/agents/platform_extensions/summon.rs | 12 +-- .../src/recipe/read_recipe_file_content.rs | 18 ++-- crates/goose/src/skills/mod.rs | 2 +- crates/goose/src/skills/supporting_files.rs | 88 ++++++++++++++++--- crates/goose/src/sources.rs | 6 +- 5 files changed, 90 insertions(+), 36 deletions(-) diff --git a/crates/goose/src/agents/platform_extensions/summon.rs b/crates/goose/src/agents/platform_extensions/summon.rs index 7ec42ccbd..da9f9fe00 100644 --- a/crates/goose/src/agents/platform_extensions/summon.rs +++ b/crates/goose/src/agents/platform_extensions/summon.rs @@ -249,11 +249,7 @@ fn scan_recipes_from_dir( continue; } - let content = match crate::skills::read_source_file_with_limit( - &source_dir, - Path::new(&file_name), - crate::agents::max_tool_response_size(), - ) { + let content = match crate::skills::read_source_file(&source_dir, Path::new(&file_name)) { Ok(content) => content, Err(error) => { warn!("Failed to read recipe {}: {}", path.display(), error); @@ -312,11 +308,7 @@ fn scan_agents_from_dir( continue; } - let content = match crate::skills::read_source_file_with_limit( - &source_dir, - Path::new(&file_name), - crate::agents::max_tool_response_size(), - ) { + let content = match crate::skills::read_source_file(&source_dir, Path::new(&file_name)) { Ok(c) => c, Err(e) => { warn!("Failed to read agent file {}: {}", path.display(), e); diff --git a/crates/goose/src/recipe/read_recipe_file_content.rs b/crates/goose/src/recipe/read_recipe_file_content.rs index 85c87f57e..159fe7378 100644 --- a/crates/goose/src/recipe/read_recipe_file_content.rs +++ b/crates/goose/src/recipe/read_recipe_file_content.rs @@ -26,12 +26,8 @@ pub fn read_recipe_file>(recipe_path: P) -> Result { let file_name = path .file_name() .ok_or_else(|| anyhow!("Recipe path has no file name: {}", path.display()))?; - let content = crate::skills::read_source_file_with_limit( - &parent_dir, - Path::new(file_name), - crate::agents::max_tool_response_size(), - ) - .map_err(|e| anyhow!("Failed to read recipe file {}: {}", path.display(), e))?; + let content = crate::skills::read_source_file(&parent_dir, Path::new(file_name)) + .map_err(|e| anyhow!("Failed to read recipe file {}: {}", path.display(), e))?; let file_path = parent_dir.join(file_name); Ok(RecipeFile { @@ -121,4 +117,14 @@ mod tests { assert!(read_recipe_file(linked_recipe).is_err()); } + + #[test] + fn recipe_above_default_tool_response_threshold_is_allowed() { + let temp_dir = TempDir::new().unwrap(); + let recipe_path = temp_dir.path().join("large.yaml"); + let content = "x".repeat(200_001); + std::fs::write(&recipe_path, &content).unwrap(); + + assert_eq!(read_recipe_file(recipe_path).unwrap().content, content); + } } diff --git a/crates/goose/src/skills/mod.rs b/crates/goose/src/skills/mod.rs index 0d3128bdc..2531305c4 100644 --- a/crates/goose/src/skills/mod.rs +++ b/crates/goose/src/skills/mod.rs @@ -8,7 +8,7 @@ pub mod client; mod supporting_files; pub use client::{SkillsClient, EXTENSION_NAME}; -pub(crate) use supporting_files::{load_supporting_file, read_source_file_with_limit}; +pub(crate) use supporting_files::{load_supporting_file, read_source_file}; use crate::config::{paths::Paths, Config}; use crate::plugins::installed_plugin_skill_dirs; diff --git a/crates/goose/src/skills/supporting_files.rs b/crates/goose/src/skills/supporting_files.rs index f13a74447..d383f0b10 100644 --- a/crates/goose/src/skills/supporting_files.rs +++ b/crates/goose/src/skills/supporting_files.rs @@ -5,6 +5,13 @@ use std::path::{Component, Path}; const LOADED_FILE_PREFIX: &str = "# Loaded: "; const LOADED_FILE_SEPARATOR: &str = "\n\n"; const LOADED_FILE_SUFFIX: &str = "\n\n---\nFile loaded into context."; +const MAX_SOURCE_FILE_BYTES: usize = crate::scheduler::MAX_SCHEDULE_RECIPE_BYTES as usize; + +#[derive(Clone, Copy)] +enum ReadLimit { + Characters(usize), + Bytes(usize), +} pub(crate) fn load_supporting_file( skill_dir: &Path, @@ -53,12 +60,27 @@ fn read_supporting_file_with_limit( read_supporting_file_with_hook(skill_dir, relative, max_characters, |_| {}) } -pub(crate) fn read_source_file_with_limit( - source_dir: &Path, +pub(crate) fn read_source_file(source_dir: &Path, relative: &Path) -> io::Result { + read_confined_file_with_hook( + source_dir, + relative, + ReadLimit::Bytes(MAX_SOURCE_FILE_BYTES), + |_| {}, + ) +} + +fn read_supporting_file_with_hook( + skill_dir: &Path, relative: &Path, max_characters: usize, + after_opened_component: impl FnMut(&Path), ) -> io::Result { - read_supporting_file_with_limit(source_dir, relative, max_characters) + read_confined_file_with_hook( + skill_dir, + relative, + ReadLimit::Characters(max_characters), + after_opened_component, + ) } fn max_utf8_bytes(max_characters: usize) -> io::Result { @@ -94,6 +116,24 @@ fn read_utf8_with_limit(mut reader: impl io::Read, max_characters: usize) -> io: Ok(content) } +fn read_utf8_with_byte_limit(mut reader: impl io::Read, max_bytes: usize) -> io::Result { + let read_size = max_bytes.checked_add(1).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "configured source file size limit is too large", + ) + })?; + let mut bytes = Vec::new(); + reader + .by_ref() + .take(read_size as u64) + .read_to_end(&mut bytes)?; + if bytes.len() > max_bytes { + return Err(file_encoding_too_large(max_bytes)); + } + String::from_utf8(bytes).map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error)) +} + fn file_too_large(max_characters: usize) -> io::Error { io::Error::new( io::ErrorKind::InvalidData, @@ -108,12 +148,18 @@ fn file_encoding_too_large(max_bytes: usize) -> io::Error { ) } -fn read_opened_file(file: fs::File, max_characters: usize) -> io::Result { - let max_bytes = max_utf8_bytes(max_characters)?; +fn read_opened_file(file: fs::File, limit: ReadLimit) -> io::Result { + let max_bytes = match limit { + ReadLimit::Characters(max_characters) => max_utf8_bytes(max_characters)?, + ReadLimit::Bytes(max_bytes) => max_bytes, + }; if file.metadata()?.len() > max_bytes as u64 { return Err(file_encoding_too_large(max_bytes)); } - read_utf8_with_limit(file, max_characters) + match limit { + ReadLimit::Characters(max_characters) => read_utf8_with_limit(file, max_characters), + ReadLimit::Bytes(max_bytes) => read_utf8_with_byte_limit(file, max_bytes), + } } fn validated_relative_components(path: &Path) -> io::Result> { @@ -221,10 +267,10 @@ fn open_skill_root( } #[cfg(unix)] -fn read_supporting_file_with_hook( +fn read_confined_file_with_hook( skill_dir: &Path, relative: &Path, - max_characters: usize, + limit: ReadLimit, mut after_opened_component: impl FnMut(&Path), ) -> io::Result { let components = validated_relative_components(relative)?; @@ -250,7 +296,7 @@ fn read_supporting_file_with_hook( )); } - read_opened_file(file, max_characters) + read_opened_file(file, limit) } #[cfg(unix)] @@ -348,10 +394,10 @@ fn open_skill_root( } #[cfg(windows)] -fn read_supporting_file_with_hook( +fn read_confined_file_with_hook( skill_dir: &Path, relative: &Path, - max_characters: usize, + limit: ReadLimit, mut after_opened_component: impl FnMut(&Path), ) -> io::Result { let components = validated_relative_components(relative)?; @@ -381,7 +427,7 @@ fn read_supporting_file_with_hook( )); } - read_opened_file(file, max_characters) + read_opened_file(file, limit) } #[cfg(windows)] @@ -479,10 +525,10 @@ fn windows_nt_status_error(status: winapi::shared::ntdef::NTSTATUS) -> io::Error } #[cfg(not(any(unix, windows)))] -fn read_supporting_file_with_hook( +fn read_confined_file_with_hook( _skill_dir: &Path, relative: &Path, - _max_characters: usize, + _limit: ReadLimit, _after_opened_component: impl FnMut(&Path), ) -> io::Result { validated_relative_components(relative)?; @@ -515,6 +561,20 @@ mod tests { assert_eq!(content, "nested guidance"); } + #[cfg(any(unix, windows))] + #[test] + fn source_file_safety_limit_is_independent() { + let root = tempfile::tempdir().unwrap(); + let source_dir = fs::canonicalize(root.path()).unwrap(); + fs::write( + source_dir.join("source.md"), + "x".repeat(MAX_SOURCE_FILE_BYTES + 1), + ) + .unwrap(); + + assert!(read_source_file(&source_dir, Path::new("source.md")).is_err()); + } + #[cfg(all( unix, any( diff --git a/crates/goose/src/sources.rs b/crates/goose/src/sources.rs index 948a0534a..038396a10 100644 --- a/crates/goose/src/sources.rs +++ b/crates/goose/src/sources.rs @@ -86,11 +86,7 @@ fn read_source_path(path: &Path) -> std::io::Result { "source path has no file name", ) })?; - crate::skills::read_source_file_with_limit( - &parent, - Path::new(file_name), - crate::agents::max_tool_response_size(), - ) + crate::skills::read_source_file(&parent, Path::new(file_name)) } fn build_source_markdown(