From 63e23c06c56e3ba2217b3ada35bb5af596dcfedf Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 28 Dec 2024 21:55:35 +0000 Subject: [PATCH 1/4] Fix FileReadTool infinite loop by maintaining original schema Co-Authored-By: Joe Moura --- src/crewai_tools/tools/file_read_tool/file_read_tool.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/crewai_tools/tools/file_read_tool/file_read_tool.py b/src/crewai_tools/tools/file_read_tool/file_read_tool.py index fe34c9d8b..8a6c2e2d8 100644 --- a/src/crewai_tools/tools/file_read_tool/file_read_tool.py +++ b/src/crewai_tools/tools/file_read_tool/file_read_tool.py @@ -27,7 +27,6 @@ class FileReadTool(BaseTool): if file_path is not None: self.file_path = file_path self.description = f"A tool that can be used to read {file_path}'s content." - self.args_schema = FixedFileReadToolSchema self._generate_description() def _run( From 5e2c38c34933aba3cfd91106a58b26d13d98545c Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 28 Dec 2024 22:26:37 +0000 Subject: [PATCH 2/4] Improve FileReadTool error handling and validation Co-Authored-By: Joe Moura --- .../tools/file_read_tool/file_read_tool.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/crewai_tools/tools/file_read_tool/file_read_tool.py b/src/crewai_tools/tools/file_read_tool/file_read_tool.py index 8a6c2e2d8..32db13f21 100644 --- a/src/crewai_tools/tools/file_read_tool/file_read_tool.py +++ b/src/crewai_tools/tools/file_read_tool/file_read_tool.py @@ -4,13 +4,7 @@ from crewai.tools import BaseTool from pydantic import BaseModel, Field -class FixedFileReadToolSchema(BaseModel): - """Input for FileReadTool.""" - - pass - - -class FileReadToolSchema(FixedFileReadToolSchema): +class FileReadToolSchema(BaseModel): """Input for FileReadTool.""" file_path: str = Field(..., description="Mandatory file full path to read the file") @@ -33,9 +27,16 @@ class FileReadTool(BaseTool): self, **kwargs: Any, ) -> Any: + file_path = kwargs.get("file_path", self.file_path) + if file_path is None: + return "Error: No file path provided. Please provide a file path either in the constructor or as an argument." + try: - file_path = kwargs.get("file_path", self.file_path) with open(file_path, "r") as file: return file.read() + except FileNotFoundError: + return f"Error: File not found at path: {file_path}" + except PermissionError: + return f"Error: Permission denied when trying to read file: {file_path}" except Exception as e: - return f"Fail to read the file {file_path}. Error: {e}" + return f"Error: Failed to read file {file_path}. {str(e)}" From aaf2641cc82e03324ce19c288c256269c5c18042 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 28 Dec 2024 22:29:46 +0000 Subject: [PATCH 3/4] Add comprehensive tests for FileReadTool Co-Authored-By: Joe Moura --- tests/file_read_tool_test.py | 84 ++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 tests/file_read_tool_test.py diff --git a/tests/file_read_tool_test.py b/tests/file_read_tool_test.py new file mode 100644 index 000000000..4646df24c --- /dev/null +++ b/tests/file_read_tool_test.py @@ -0,0 +1,84 @@ +import os +import pytest +from crewai_tools import FileReadTool + +def test_file_read_tool_constructor(): + """Test FileReadTool initialization with file_path.""" + # Create a temporary test file + test_file = "/tmp/test_file.txt" + test_content = "Hello, World!" + with open(test_file, "w") as f: + f.write(test_content) + + # Test initialization with file_path + tool = FileReadTool(file_path=test_file) + assert tool.file_path == test_file + assert "test_file.txt" in tool.description + + # Clean up + os.remove(test_file) + +def test_file_read_tool_run(): + """Test FileReadTool _run method with file_path at runtime.""" + # Create a temporary test file + test_file = "/tmp/test_file.txt" + test_content = "Hello, World!" + with open(test_file, "w") as f: + f.write(test_content) + + # Test reading file with runtime file_path + tool = FileReadTool() + result = tool._run(file_path=test_file) + assert result == test_content + + # Clean up + os.remove(test_file) + +def test_file_read_tool_error_handling(): + """Test FileReadTool error handling.""" + # Test missing file path + tool = FileReadTool() + result = tool._run() + assert "Error: No file path provided" in result + + # Test non-existent file + result = tool._run(file_path="/nonexistent/file.txt") + assert "Error: File not found at path:" in result + + # Test permission error (create a file without read permissions) + test_file = "/tmp/no_permission.txt" + with open(test_file, "w") as f: + f.write("test") + os.chmod(test_file, 0o000) + + result = tool._run(file_path=test_file) + assert "Error: Permission denied" in result + + # Clean up + os.chmod(test_file, 0o666) # Restore permissions to delete + os.remove(test_file) + +def test_file_read_tool_constructor_and_run(): + """Test FileReadTool using both constructor and runtime file paths.""" + # Create two test files + test_file1 = "/tmp/test1.txt" + test_file2 = "/tmp/test2.txt" + content1 = "File 1 content" + content2 = "File 2 content" + + with open(test_file1, "w") as f1, open(test_file2, "w") as f2: + f1.write(content1) + f2.write(content2) + + # Test that constructor file_path works + tool = FileReadTool(file_path=test_file1) + result = tool._run() + assert result == content1 + + # Test that runtime file_path overrides constructor + result = tool._run(file_path=test_file2) + assert result == content2 + + # Clean up + os.remove(test_file1) + os.remove(test_file2) From d3391d9ba4c1b3696dbbe7188aa59e6dc6ce8761 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 28 Dec 2024 23:10:51 +0000 Subject: [PATCH 4/4] Add comprehensive documentation and type hints to FileReadTool Co-Authored-By: Joe Moura --- .../tools/file_read_tool/file_read_tool.py | 49 +++++++++++++++++-- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/src/crewai_tools/tools/file_read_tool/file_read_tool.py b/src/crewai_tools/tools/file_read_tool/file_read_tool.py index 32db13f21..323a26d51 100644 --- a/src/crewai_tools/tools/file_read_tool/file_read_tool.py +++ b/src/crewai_tools/tools/file_read_tool/file_read_tool.py @@ -11,22 +11,49 @@ class FileReadToolSchema(BaseModel): class FileReadTool(BaseTool): + """A tool for reading file contents. + + This tool inherits its schema handling from BaseTool to avoid recursive schema + definition issues. The args_schema is set to FileReadToolSchema which defines + the required file_path parameter. The schema should not be overridden in the + constructor as it would break the inheritance chain and cause infinite loops. + + The tool supports two ways of specifying the file path: + 1. At construction time via the file_path parameter + 2. At runtime via the file_path parameter in the tool's input + + Args: + file_path (Optional[str]): Path to the file to be read. If provided, + this becomes the default file path for the tool. + **kwargs: Additional keyword arguments passed to BaseTool. + + Example: + >>> tool = FileReadTool(file_path="/path/to/file.txt") + >>> content = tool.run() # Reads /path/to/file.txt + >>> content = tool.run(file_path="/path/to/other.txt") # Reads other.txt + """ name: str = "Read a file's content" - description: str = "A tool that can be used to read a file's content." + description: str = "A tool that reads the content of a file. To use this tool, provide a 'file_path' parameter with the path to the file you want to read." args_schema: Type[BaseModel] = FileReadToolSchema file_path: Optional[str] = None - def __init__(self, file_path: Optional[str] = None, **kwargs): + def __init__(self, file_path: Optional[str] = None, **kwargs: Any) -> None: + """Initialize the FileReadTool. + + Args: + file_path (Optional[str]): Path to the file to be read. If provided, + this becomes the default file path for the tool. + **kwargs: Additional keyword arguments passed to BaseTool. + """ super().__init__(**kwargs) if file_path is not None: self.file_path = file_path - self.description = f"A tool that can be used to read {file_path}'s content." - self._generate_description() + self.description = f"A tool that reads file content. The default file is {file_path}, but you can provide a different 'file_path' parameter to read another file." def _run( self, **kwargs: Any, - ) -> Any: + ) -> str: file_path = kwargs.get("file_path", self.file_path) if file_path is None: return "Error: No file path provided. Please provide a file path either in the constructor or as an argument." @@ -40,3 +67,15 @@ class FileReadTool(BaseTool): return f"Error: Permission denied when trying to read file: {file_path}" except Exception as e: return f"Error: Failed to read file {file_path}. {str(e)}" + + def _generate_description(self) -> None: + """Generate the tool description based on file path. + + This method updates the tool's description to include information about + the default file path while maintaining the ability to specify a different + file at runtime. + + Returns: + None + """ + self.description = f"A tool that can be used to read {self.file_path}'s content."