fix: decouple source file and tool response limits (#11391)
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -26,12 +26,8 @@ pub fn read_recipe_file<P: AsRef<Path>>(recipe_path: P) -> Result<RecipeFile> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<String> {
|
||||
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<String> {
|
||||
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<usize> {
|
||||
@@ -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<String> {
|
||||
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<String> {
|
||||
let max_bytes = max_utf8_bytes(max_characters)?;
|
||||
fn read_opened_file(file: fs::File, limit: ReadLimit) -> io::Result<String> {
|
||||
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<Vec<&std::ffi::OsStr>> {
|
||||
@@ -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<String> {
|
||||
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<String> {
|
||||
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<String> {
|
||||
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(
|
||||
|
||||
@@ -86,11 +86,7 @@ fn read_source_path(path: &Path) -> std::io::Result<String> {
|
||||
"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(
|
||||
|
||||
Reference in New Issue
Block a user