Skip to content

Commit 33728ff

Browse files
committed
Changed: serdesAI allowed tools constructor signatures for better ergonomics and error handling
- Refactored all allowed tool constructors (ReadTool, EditTool, WriteTool, GrepTool, GlobTool) to accept flexible iterator inputs via 'impl IntoIterator<Item = impl AsRef<Path>>' instead of 'Vec<PathBuf>' - All constructors now return 'ToolResult<Self>' instead of 'Self' to provide proper error handling for canonicalization failures - Replaced 'AllowedPathResolver::from_canonical()' with 'AllowedPathResolver::new()' for consistent error propagation - Updated test fixtures to use array literal syntax '[dir.path()]' instead of vector syntax 'vec![dir.path().to_path_buf()]' for improved readability - Aligned serdesAI tool signatures with corresponding rig framework implementations for consistency
1 parent a988885 commit 33728ff

5 files changed

Lines changed: 45 additions & 36 deletions

File tree

src/llm-coding-tools-serdesai/src/allowed/edit.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use serde::Deserialize;
88
use serdes_ai::tools::{
99
RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult, ToolReturn,
1010
};
11-
use std::path::PathBuf;
11+
use std::path::Path;
1212

1313
use crate::convert::edit_error_to_serdes;
1414

@@ -33,12 +33,15 @@ pub struct EditTool {
3333
}
3434

3535
impl EditTool {
36-
/// Creates a new edit tool restricted to the given canonical directories.
37-
/// `allowed_directories` should already be canonicalized (see [`AllowedPathResolver::from_canonical`]).
38-
pub fn new(allowed_directories: Vec<PathBuf>) -> Self {
39-
Self {
40-
resolver: AllowedPathResolver::from_canonical(allowed_directories),
41-
}
36+
/// Creates a new edit tool restricted to the given directories.
37+
///
38+
/// Returns an error if any directory doesn't exist or can't be canonicalized.
39+
pub fn new(
40+
allowed_paths: impl IntoIterator<Item = impl AsRef<Path>>,
41+
) -> llm_coding_tools_core::ToolResult<Self> {
42+
Ok(Self {
43+
resolver: AllowedPathResolver::new(allowed_paths)?,
44+
})
4245
}
4346
}
4447

@@ -110,7 +113,7 @@ mod tests {
110113
let dir = TempDir::new().unwrap();
111114
std::fs::write(dir.path().join("test.txt"), "hello world").unwrap();
112115

113-
let tool = EditTool::new(vec![dir.path().to_path_buf()]);
116+
let tool = EditTool::new([dir.path()]).unwrap();
114117
let result = tool
115118
.call(
116119
&mock_ctx(),
@@ -130,7 +133,7 @@ mod tests {
130133
#[tokio::test]
131134
async fn rejects_path_traversal() {
132135
let dir = TempDir::new().unwrap();
133-
let tool = EditTool::new(vec![dir.path().to_path_buf()]);
136+
let tool = EditTool::new([dir.path()]).unwrap();
134137
let result = tool
135138
.call(
136139
&mock_ctx(),

src/llm-coding-tools-serdesai/src/allowed/glob.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use llm_coding_tools_core::path::AllowedPathResolver;
66
use llm_coding_tools_core::{ToolContext, ToolOutput};
77
use serde::Deserialize;
88
use serdes_ai::tools::{RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult};
9-
use std::path::PathBuf;
9+
use std::path::Path;
1010

1111
use crate::convert::to_serdes_result;
1212

@@ -27,10 +27,12 @@ pub struct GlobTool {
2727

2828
impl GlobTool {
2929
/// Creates a new glob tool restricted to the given directories.
30-
pub fn new(allowed_directories: Vec<PathBuf>) -> Self {
31-
Self {
32-
resolver: AllowedPathResolver::from_canonical(allowed_directories),
33-
}
30+
pub fn new(
31+
allowed_paths: impl IntoIterator<Item = impl AsRef<Path>>,
32+
) -> llm_coding_tools_core::ToolResult<Self> {
33+
Ok(Self {
34+
resolver: AllowedPathResolver::new(allowed_paths)?,
35+
})
3436
}
3537
}
3638

@@ -108,7 +110,7 @@ mod tests {
108110
fs::create_dir_all(dir.path().join("src")).unwrap();
109111
File::create(dir.path().join("src/lib.rs")).unwrap();
110112

111-
let tool = GlobTool::new(vec![dir.path().to_path_buf()]);
113+
let tool = GlobTool::new([dir.path()]).unwrap();
112114
let result = tool
113115
.call(
114116
&mock_ctx(),
@@ -127,7 +129,7 @@ mod tests {
127129
#[tokio::test]
128130
async fn rejects_path_traversal() {
129131
let dir = TempDir::new().unwrap();
130-
let tool = GlobTool::new(vec![dir.path().to_path_buf()]);
132+
let tool = GlobTool::new([dir.path()]).unwrap();
131133
let result = tool
132134
.call(
133135
&mock_ctx(),

src/llm-coding-tools-serdesai/src/allowed/grep.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use serdes_ai::tools::{
99
RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult, ToolReturn,
1010
};
1111
use std::fmt::Write;
12-
use std::path::PathBuf;
12+
use std::path::Path;
1313

1414
use crate::convert::to_serdes_result;
1515

@@ -40,10 +40,12 @@ pub struct GrepTool<const LINE_NUMBERS: bool = true> {
4040

4141
impl<const LINE_NUMBERS: bool> GrepTool<LINE_NUMBERS> {
4242
/// Creates a new grep tool restricted to the given directories.
43-
pub fn new(allowed_directories: Vec<PathBuf>) -> Self {
44-
Self {
45-
resolver: AllowedPathResolver::from_canonical(allowed_directories),
46-
}
43+
pub fn new(
44+
allowed_paths: impl IntoIterator<Item = impl AsRef<Path>>,
45+
) -> llm_coding_tools_core::ToolResult<Self> {
46+
Ok(Self {
47+
resolver: AllowedPathResolver::new(allowed_paths)?,
48+
})
4749
}
4850
}
4951

@@ -179,7 +181,7 @@ mod tests {
179181
let dir = TempDir::new().unwrap();
180182
std::fs::write(dir.path().join("test.txt"), "hello world").unwrap();
181183

182-
let tool: GrepTool<true> = GrepTool::new(vec![dir.path().to_path_buf()]);
184+
let tool: GrepTool<true> = GrepTool::new([dir.path()]).unwrap();
183185
let result = tool
184186
.call(
185187
&mock_ctx(),
@@ -199,7 +201,7 @@ mod tests {
199201
#[tokio::test]
200202
async fn rejects_path_traversal() {
201203
let dir = TempDir::new().unwrap();
202-
let tool: GrepTool<true> = GrepTool::new(vec![dir.path().to_path_buf()]);
204+
let tool: GrepTool<true> = GrepTool::new([dir.path()]).unwrap();
203205
let result = tool
204206
.call(
205207
&mock_ctx(),
@@ -216,7 +218,7 @@ mod tests {
216218
#[tokio::test]
217219
async fn rejects_empty_pattern() {
218220
let dir = TempDir::new().unwrap();
219-
let tool: GrepTool<true> = GrepTool::new(vec![dir.path().to_path_buf()]);
221+
let tool: GrepTool<true> = GrepTool::new([dir.path()]).unwrap();
220222
let result = tool
221223
.call(
222224
&mock_ctx(),

src/llm-coding-tools-serdesai/src/allowed/read.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use llm_coding_tools_core::path::AllowedPathResolver;
66
use llm_coding_tools_core::{ToolContext, ToolResult as CoreToolResult};
77
use serde::Deserialize;
88
use serdes_ai::tools::{RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult};
9-
use std::path::PathBuf;
9+
use std::path::Path;
1010

1111
use crate::convert::to_serdes_result;
1212

@@ -46,9 +46,9 @@ impl<const LINE_NUMBERS: bool> ReadTool<LINE_NUMBERS> {
4646
/// Creates a new read tool restricted to the given directories.
4747
///
4848
/// Returns an error if any directory doesn't exist or can't be canonicalized.
49-
pub fn new(allowed_directories: Vec<PathBuf>) -> CoreToolResult<Self> {
49+
pub fn new(allowed_paths: impl IntoIterator<Item = impl AsRef<Path>>) -> CoreToolResult<Self> {
5050
Ok(Self {
51-
resolver: AllowedPathResolver::new(allowed_directories)?,
51+
resolver: AllowedPathResolver::new(allowed_paths)?,
5252
})
5353
}
5454
}
@@ -124,7 +124,7 @@ mod tests {
124124
let dir = TempDir::new().unwrap();
125125
std::fs::write(dir.path().join("test.txt"), "hello\nworld\n").unwrap();
126126

127-
let tool: ReadTool<true> = ReadTool::new(vec![dir.path().to_path_buf()]).unwrap();
127+
let tool: ReadTool<true> = ReadTool::new([dir.path()]).unwrap();
128128
let result = tool
129129
.call(
130130
&mock_ctx(),
@@ -145,7 +145,7 @@ mod tests {
145145
#[tokio::test]
146146
async fn rejects_path_traversal() {
147147
let dir = TempDir::new().unwrap();
148-
let tool: ReadTool<true> = ReadTool::new(vec![dir.path().to_path_buf()]).unwrap();
148+
let tool: ReadTool<true> = ReadTool::new([dir.path()]).unwrap();
149149
let result = tool
150150
.call(
151151
&mock_ctx(),

src/llm-coding-tools-serdesai/src/allowed/write.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use llm_coding_tools_core::path::AllowedPathResolver;
66
use llm_coding_tools_core::{ToolContext, ToolOutput};
77
use serde::Deserialize;
88
use serdes_ai::tools::{RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult};
9-
use std::path::PathBuf;
9+
use std::path::Path;
1010

1111
use crate::convert::to_serdes_result;
1212

@@ -26,10 +26,12 @@ pub struct WriteTool {
2626

2727
impl WriteTool {
2828
/// Creates a new write tool restricted to the given directories.
29-
pub fn new(allowed_directories: Vec<PathBuf>) -> Self {
30-
Self {
31-
resolver: AllowedPathResolver::from_canonical(allowed_directories),
32-
}
29+
pub fn new(
30+
allowed_paths: impl IntoIterator<Item = impl AsRef<Path>>,
31+
) -> llm_coding_tools_core::ToolResult<Self> {
32+
Ok(Self {
33+
resolver: AllowedPathResolver::new(allowed_paths)?,
34+
})
3335
}
3436
}
3537

@@ -85,7 +87,7 @@ mod tests {
8587
#[tokio::test]
8688
async fn writes_new_file() {
8789
let dir = TempDir::new().unwrap();
88-
let tool = WriteTool::new(vec![dir.path().to_path_buf()]);
90+
let tool = WriteTool::new([dir.path()]).unwrap();
8991
let result = tool
9092
.call(
9193
&mock_ctx(),
@@ -105,7 +107,7 @@ mod tests {
105107
#[tokio::test]
106108
async fn rejects_path_traversal() {
107109
let dir = TempDir::new().unwrap();
108-
let tool = WriteTool::new(vec![dir.path().to_path_buf()]);
110+
let tool = WriteTool::new([dir.path()]).unwrap();
109111
let result = tool
110112
.call(
111113
&mock_ctx(),

0 commit comments

Comments
 (0)