Fixes #54244 When the `read_file` tool is called with a path that points to a directory instead of a file, it now returns a clear, actionable error message telling the agent to use `list_directory` instead. Previously the tool would fail with an unhelpful generic error. Now it explicitly checks whether the path is a directory before attempting to read it. A test covering this case is also included. Release Notes: - Fixed `read_file` tool returning an unhelpful error when given a directory path; it now suggests using `list_directory` instead. ---------
Unified · Split
| @@ -184,6 +184,13 @@ impl AgentTool for ReadFileTool { · 183 unmodified lines | |||
| 184 | anyhow::Ok(()) | 184 | anyhow::Ok(()) |
| 185 | }).map_err(tool_content_err)?; | 185 | }).map_err(tool_content_err)?; |
| 186 | 186 | ||
| 187 | if fs.is_dir(&abs_path).await { | ||
| 188 | return Err(tool_content_err(format!( | ||
| 189 | "{} is a directory, not a file. Use the list_directory tool to explore directory contents.", | ||
| 190 | &input.path | ||
| 191 | ))); | ||
| 192 | } | ||
| 193 | |||
| 187 | if let Some(canonical_target) = &symlink_canonical_target { | 194 | if let Some(canonical_target) = &symlink_canonical_target { |
| 188 | let authorize = cx.update(|cx| { | 195 | let authorize = cx.update(|cx| { |
| 189 | authorize_symlink_access( | 196 | authorize_symlink_access( |
| @@ -356,6 +363,39 @@ mod test { · 166 unmodified lines | |||
| 356 | use std::sync::Arc; | 363 | use std::sync::Arc; |
| 357 | use util::path; | 364 | use util::path; |
| 358 | 365 | ||
| 366 | #[gpui::test] | ||
| 367 | async fn test_read_directory_path(cx: &mut TestAppContext) { | ||
| 368 | init_test(cx); | ||
| 369 | |||
| 370 | let fs = FakeFs::new(cx.executor()); | ||
| 371 | fs.insert_tree( | ||
| 372 | path!("/root"), | ||
| 373 | json!({ | ||
| 374 | "some_dir": {} | ||
| 375 | }), | ||
| 376 | ) | ||
| 377 | .await; | ||
| 378 | let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await; | ||
| 379 | let action_log = cx.new(|_| ActionLog::new(project.clone())); | ||
| 380 | let tool = Arc::new(ReadFileTool::new(project, action_log, true)); | ||
| 381 | let (event_stream, _) = ToolCallEventStream::test(); | ||
| 382 | |||
| 383 | let result = cx | ||
| 384 | .update(|cx| { | ||
| 385 | let input = ReadFileToolInput { | ||
| 386 | path: "root/some_dir".to_string(), | ||
| 387 | start_line: None, | ||
| 388 | end_line: None, | ||
| 389 | }; | ||
| 390 | tool.run(ToolInput::resolved(input), event_stream, cx) | ||
| 391 | }) | ||
| 392 | .await; | ||
| 393 | assert_eq!( | ||
| 394 | error_text(result.unwrap_err()), | ||
| 395 | "root/some_dir is a directory, not a file. Use the list_directory tool to explore directory contents." | ||
| 396 | ); | ||
| 397 | } | ||
| 398 | |||
| 359 | #[gpui::test] | 399 | #[gpui::test] |
| 360 | async fn test_read_nonexistent_file(cx: &mut TestAppContext) { | 400 | async fn test_read_nonexistent_file(cx: &mut TestAppContext) { |
| 361 | init_test(cx); | 401 | init_test(cx); |