Skip to content

Commit 5c621df

Browse files
dingfelidingfeli
authored
Fix: error from merge for fs read (#2225)
1 parent b1d9517 commit 5c621df

File tree

2 files changed

+116
-101
lines changed

2 files changed

+116
-101
lines changed

‎crates/chat-cli/src/cli/agent.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ impl Agent {
186186
}
187187
}
188188

189-
#[derive(Debug)]
189+
#[derive(Debug, PartialEq)]
190190
pub enum PermissionEvalResult {
191191
Allow,
192192
Ask,

‎crates/chat-cli/src/cli/chat/tools/fs_read.rs

Lines changed: 115 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,121 @@ impl FsRead {
9999
}
100100
}
101101

102+
pub fn eval_perm(&self, agent: &Agent) -> PermissionEvalResult {
103+
#[derive(Debug, Deserialize)]
104+
#[serde(rename_all = "camelCase")]
105+
struct Settings {
106+
#[serde(default)]
107+
allowed_paths: Vec<String>,
108+
#[serde(default)]
109+
denied_paths: Vec<String>,
110+
#[serde(default = "default_allow_read_only")]
111+
allow_read_only: bool,
112+
}
113+
114+
fn default_allow_read_only() -> bool {
115+
true
116+
}
117+
118+
let is_in_allowlist = agent.allowed_tools.contains("fs_read");
119+
match agent.tools_settings.get("fs_read") {
120+
Some(settings) if is_in_allowlist => {
121+
let Settings {
122+
allowed_paths,
123+
denied_paths,
124+
allow_read_only,
125+
} = match serde_json::from_value::<Settings>(settings.clone()) {
126+
Ok(settings) => settings,
127+
Err(e) => {
128+
error!("Failed to deserialize tool settings for fs_read: {:?}", e);
129+
return PermissionEvalResult::Ask;
130+
},
131+
};
132+
let allow_set = {
133+
let mut builder = GlobSetBuilder::new();
134+
for path in &allowed_paths {
135+
if let Ok(glob) = Glob::new(path) {
136+
builder.add(glob);
137+
} else {
138+
warn!("Failed to create glob from path given: {path}. Ignoring.");
139+
}
140+
}
141+
builder.build()
142+
};
143+
144+
let deny_set = {
145+
let mut builder = GlobSetBuilder::new();
146+
for path in &denied_paths {
147+
if let Ok(glob) = Glob::new(path) {
148+
builder.add(glob);
149+
} else {
150+
warn!("Failed to create glob from path given: {path}. Ignoring.");
151+
}
152+
}
153+
builder.build()
154+
};
155+
156+
match (allow_set, deny_set) {
157+
(Ok(allow_set), Ok(deny_set)) => {
158+
let eval_res = self
159+
.operations
160+
.iter()
161+
.map(|op| {
162+
match op {
163+
FsReadOperation::Line(FsLine { path, .. })
164+
| FsReadOperation::Directory(FsDirectory { path, .. })
165+
| FsReadOperation::Search(FsSearch { path, .. }) => {
166+
if deny_set.is_match(path) {
167+
return PermissionEvalResult::Deny;
168+
}
169+
if allow_set.is_match(path) {
170+
return PermissionEvalResult::Allow;
171+
}
172+
},
173+
FsReadOperation::Image(fs_image) => {
174+
let paths = &fs_image.image_paths;
175+
if paths.iter().any(|path| deny_set.is_match(path)) {
176+
return PermissionEvalResult::Deny;
177+
}
178+
if paths.iter().all(|path| allow_set.is_match(path)) {
179+
return PermissionEvalResult::Allow;
180+
}
181+
},
182+
}
183+
184+
if allow_read_only {
185+
PermissionEvalResult::Allow
186+
} else {
187+
PermissionEvalResult::Ask
188+
}
189+
})
190+
.collect::<Vec<_>>();
191+
192+
if eval_res.contains(&PermissionEvalResult::Deny) {
193+
PermissionEvalResult::Deny
194+
} else if eval_res.contains(&PermissionEvalResult::Ask) {
195+
PermissionEvalResult::Ask
196+
} else {
197+
PermissionEvalResult::Allow
198+
}
199+
},
200+
(allow_res, deny_res) => {
201+
if let Err(e) = allow_res {
202+
warn!("fs_read failed to build allow set: {:?}", e);
203+
}
204+
if let Err(e) = deny_res {
205+
warn!("fs_read failed to build deny set: {:?}", e);
206+
}
207+
warn!("One or more detailed args failed to parse, falling back to ask");
208+
PermissionEvalResult::Ask
209+
},
210+
}
211+
},
212+
None if is_in_allowlist => PermissionEvalResult::Allow,
213+
_ => PermissionEvalResult::Ask,
214+
}
215+
}
216+
102217
pub async fn invoke(&self, os: &Os, updates: &mut impl Write) -> Result<InvokeOutput> {
103218
if self.operations.len() == 1 {
104219
// Single operation - return result directly
@@ -218,106 +333,6 @@ impl FsReadOperation {
218333
FsReadOperation::Image(fs_image) => fs_image.invoke(updates).await,
219334
}
220335
}
221-
222-
pub fn eval_perm(&self, agent: &Agent) -> PermissionEvalResult {
223-
#[derive(Debug, Deserialize)]
224-
#[serde(rename_all = "camelCase")]
225-
struct Settings {
226-
#[serde(default)]
227-
allowed_paths: Vec<String>,
228-
#[serde(default)]
229-
denied_paths: Vec<String>,
230-
#[serde(default = "default_allow_read_only")]
231-
allow_read_only: bool,
232-
}
233-
234-
fn default_allow_read_only() -> bool {
235-
true
236-
}
237-
238-
let is_in_allowlist = agent.allowed_tools.contains("fs_read");
239-
match agent.tools_settings.get("fs_read") {
240-
Some(settings) if is_in_allowlist => {
241-
let Settings {
242-
allowed_paths,
243-
denied_paths,
244-
allow_read_only,
245-
} = match serde_json::from_value::<Settings>(settings.clone()) {
246-
Ok(settings) => settings,
247-
Err(e) => {
248-
error!("Failed to deserialize tool settings for fs_read: {:?}", e);
249-
return PermissionEvalResult::Ask;
250-
},
251-
};
252-
let allow_set = {
253-
let mut builder = GlobSetBuilder::new();
254-
for path in &allowed_paths {
255-
if let Ok(glob) = Glob::new(path) {
256-
builder.add(glob);
257-
} else {
258-
warn!("Failed to create glob from path given: {path}. Ignoring.");
259-
}
260-
}
261-
builder.build()
262-
};
263-
264-
let deny_set = {
265-
let mut builder = GlobSetBuilder::new();
266-
for path in &denied_paths {
267-
if let Ok(glob) = Glob::new(path) {
268-
builder.add(glob);
269-
} else {
270-
warn!("Failed to create glob from path given: {path}. Ignoring.");
271-
}
272-
}
273-
builder.build()
274-
};
275-
276-
match (allow_set, deny_set) {
277-
(Ok(allow_set), Ok(deny_set)) => {
278-
match self {
279-
Self::Line(FsLine { path, .. })
280-
| Self::Directory(FsDirectory { path, .. })
281-
| Self::Search(FsSearch { path, .. }) => {
282-
if deny_set.is_match(path) {
283-
return PermissionEvalResult::Deny;
284-
}
285-
if allow_set.is_match(path) {
286-
return PermissionEvalResult::Allow;
287-
}
288-
},
289-
Self::Image(fs_image) => {
290-
let paths = &fs_image.image_paths;
291-
if paths.iter().any(|path| deny_set.is_match(path)) {
292-
return PermissionEvalResult::Deny;
293-
}
294-
if paths.iter().all(|path| allow_set.is_match(path)) {
295-
return PermissionEvalResult::Allow;
296-
}
297-
},
298-
}
299-
if allow_read_only {
300-
PermissionEvalResult::Allow
301-
} else {
302-
PermissionEvalResult::Ask
303-
}
304-
},
305-
(allow_res, deny_res) => {
306-
if let Err(e) = allow_res {
307-
warn!("fs_read failed to build allow set: {:?}", e);
308-
}
309-
if let Err(e) = deny_res {
310-
warn!("fs_read failed to build deny set: {:?}", e);
311-
}
312-
warn!("One or more detailed args failed to parse, falling back to ask");
313-
PermissionEvalResult::Ask
314-
},
315-
}
316-
},
317-
None if is_in_allowlist => PermissionEvalResult::Allow,
318-
_ => PermissionEvalResult::Ask,
319-
}
320-
}
321336
}
322337

323338
/// Read images from given paths.

0 commit comments

Comments
 (0)