From aae5dc59b503079d4874f42b0cd8edac968b43f8 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Thu, 10 Jul 2025 00:02:05 +0200 Subject: [PATCH] Refactoring * Project.is_path_ignored -> Project.is_ignored_relative_path * SerenaAgent.path_is_gitignored -> Project.is_ignored_path * SerenaAgent.path_is_inside_project -> Project.is_path_in_project * SerenaAgent.validate_relative_path -> Project.validate_relative_path --- src/serena/agent.py | 43 ------------------------------ src/serena/project.py | 48 +++++++++++++++++++++++++++++++--- src/serena/tools/file_tools.py | 22 ++++++++-------- 3 files changed, 56 insertions(+), 57 deletions(-) diff --git a/src/serena/agent.py b/src/serena/agent.py index 0ecd671a..f05da6bf 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -32,7 +32,6 @@ from serena.project import Project from serena.prompt_factory import SerenaPromptFactory from serena.symbol import SymbolManager from serena.tools import Tool, ToolRegistry -from serena.util.file_system import match_path from solidlsp import SolidLanguageServer if TYPE_CHECKING: @@ -283,48 +282,6 @@ class SerenaAgent: raise ValueError("Cannot get project root if no project is active.") return project.project_root - def path_is_inside_project(self, path: str | Path) -> bool: - """ - Checks if the given (absolute or relative) path is inside the project directory. - Note that even relative paths may be outside if the contain ".." or point to symlinks. - """ - path = Path(path) - _proj_root = Path(self.get_project_root()) - if not path.is_absolute(): - path = _proj_root / path - - path = path.resolve() - return path.is_relative_to(_proj_root) - - # TODO: Move to project - def path_is_gitignored(self, path: str | Path) -> bool: - """ - Checks if the given path is ignored by git. Non absolute paths are assumed to be relative to the project root. - """ - path = Path(path) - if path.is_absolute(): - relative_path = path.relative_to(self.get_project_root()) - else: - relative_path = path - - # always ignore paths inside .git - if len(relative_path.parts) > 0 and relative_path.parts[0] == ".git": - return True - - project = self.get_active_project_or_raise() - return match_path(str(relative_path), project.get_ignore_spec(), root_path=self.get_project_root()) - - def validate_relative_path(self, relative_path: str) -> None: - """ - Validates that the given relative path is safe to read or edit, - meaning it's inside the project directory and is not ignored by git. - """ - if not self.path_is_inside_project(relative_path): - raise ValueError(f"{relative_path=} points to path outside of the repository root, can't use it for safety reasons") - - if self.path_is_gitignored(relative_path): - raise ValueError(f"File {relative_path} is gitignored, can't read or edit it for safety reasons") - def get_exposed_tool_instances(self) -> list["Tool"]: """ :return: the tool instances which are exposed (e.g. to the MCP client). diff --git a/src/serena/project.py b/src/serena/project.py index 4e99e0ca..a2360c31 100644 --- a/src/serena/project.py +++ b/src/serena/project.py @@ -89,7 +89,7 @@ class Project: def is_ignored_dirname(self, dirname: str) -> bool: return dirname.startswith(".") - def is_ignored_path(self, relative_path: str, ignore_unsupported_files: bool = True) -> bool: + def is_ignored_relative_path(self, relative_path: str, ignore_unsupported_files: bool = True) -> bool: """ Determine whether a path should be ignored based on file type and ignore patterns. @@ -124,6 +124,48 @@ class Project: return match_path(relative_path, self.get_ignore_spec(), root_path=self.project_root) + def is_ignored_path(self, path: str | Path) -> bool: + """ + Checks whether the given path is ignored + + :param path: the path to check, can be absolute or relative + """ + path = Path(path) + if path.is_absolute(): + relative_path = path.relative_to(self.project_root) + else: + relative_path = path + + # always ignore paths inside .git + if len(relative_path.parts) > 0 and relative_path.parts[0] == ".git": + return True + + return match_path(str(relative_path), self.get_ignore_spec(), root_path=self.project_root) + + def is_path_in_project(self, path: str | Path) -> bool: + """ + Checks if the given (absolute or relative) path is inside the project directory. + Note that even relative paths may be outside if they contain ".." or point to symlinks. + """ + path = Path(path) + _proj_root = Path(self.project_root) + if not path.is_absolute(): + path = _proj_root / path + + path = path.resolve() + return path.is_relative_to(_proj_root) + + def validate_relative_path(self, relative_path: str) -> None: + """ + Validates that the given relative path is safe to read or edit, + meaning it's inside the project directory and is not ignored by git. + """ + if not self.is_path_in_project(relative_path): + raise ValueError(f"{relative_path=} points to path outside of the repository root; cannot access for safety reasons") + + if self.is_ignored_path(relative_path): + raise ValueError(f"Path {relative_path} is ignored; cannot access for safety reasons") + def gather_source_files(self, relative_path: str = "") -> list[str]: """Retrieves relative paths of all source files, optionally limited to the given path @@ -137,11 +179,11 @@ class Project: return [relative_path] else: for root, dirs, files in os.walk(start_path, followlinks=True): - dirs[:] = [d for d in dirs if not self.is_ignored_path(os.path.join(root, d))] + dirs[:] = [d for d in dirs if not self.is_ignored_relative_path(os.path.join(root, d))] for file in files: rel_file_path = os.path.relpath(os.path.join(root, file), start=self.project_root) try: - if not self.is_ignored_path(rel_file_path): + if not self.is_ignored_relative_path(rel_file_path): rel_file_paths.append(rel_file_path) except FileNotFoundError: log.warning( diff --git a/src/serena/tools/file_tools.py b/src/serena/tools/file_tools.py index 5cff6d9e..f79ffe06 100644 --- a/src/serena/tools/file_tools.py +++ b/src/serena/tools/file_tools.py @@ -39,7 +39,7 @@ class ReadFileTool(Tool): required for the task. :return: the full text of the file at the given relative path """ - self.agent.validate_relative_path(relative_path) + self.project.validate_relative_path(relative_path) result = self.project.read_file(relative_path) result_lines = result.splitlines() @@ -73,7 +73,7 @@ class CreateTextFileTool(Tool, ToolMarkerCanEdit): :param content: the (utf-8-encoded) content to write to the file :return: a message indicating success or failure """ - self.agent.validate_relative_path(relative_path) + self.project.validate_relative_path(relative_path) abs_path = (Path(self.get_project_root()) / relative_path).resolve() will_overwrite_existing = abs_path.exists() @@ -102,14 +102,14 @@ class ListDirTool(Tool): required for the task. :return: a JSON object with the names of directories and files within the given directory """ - self.agent.validate_relative_path(relative_path) + self.project.validate_relative_path(relative_path) dirs, files = scan_directory( os.path.join(self.get_project_root(), relative_path), relative_to=self.get_project_root(), recursive=recursive, - is_ignored_dir=self.agent.path_is_gitignored, - is_ignored_file=self.agent.path_is_gitignored, + is_ignored_dir=self.project.is_ignored_path, + is_ignored_file=self.project.is_ignored_path, ) result = json.dumps({"dirs": dirs, "files": files}) @@ -129,13 +129,13 @@ class FindFileTool(Tool): :param relative_path: the relative path to the directory to search in; pass "." to scan the project root :return: a JSON object with the list of matching files """ - self.agent.validate_relative_path(relative_path) + self.project.validate_relative_path(relative_path) dir_to_scan = os.path.join(self.get_project_root(), relative_path) # find the files by ignoring everything that doesn't match def is_ignored_file(abs_path: str) -> bool: - if self.agent.path_is_gitignored(abs_path): + if self.project.is_ignored_path(abs_path): return True filename = os.path.basename(abs_path) return not fnmatch(filename, file_mask) @@ -143,7 +143,7 @@ class FindFileTool(Tool): dirs, files = scan_directory( path=dir_to_scan, recursive=True, - is_ignored_dir=self.agent.path_is_gitignored, + is_ignored_dir=self.project.is_ignored_path, is_ignored_file=is_ignored_file, relative_to=self.get_project_root(), ) @@ -185,7 +185,7 @@ class ReplaceRegexTool(Tool, ToolMarkerCanEdit): If this is set to False and the regex matches multiple occurrences, an error will be returned (and you may retry with a revised, more specific regex). """ - self.agent.validate_relative_path(relative_path) + self.project.validate_relative_path(relative_path) with EditedFileContext(relative_path, self.agent) as context: original_content = context.get_original_content() updated_content, n = re.subn(regex, repl, original_content, flags=re.DOTALL | re.MULTILINE) @@ -373,8 +373,8 @@ class SearchForPatternTool(Tool): dirs, rel_paths_to_search = scan_directory( path=abs_path, recursive=True, - is_ignored_dir=self.agent.path_is_gitignored, - is_ignored_file=self.agent.path_is_gitignored, + is_ignored_dir=self.project.is_ignored_path, + is_ignored_file=self.project.is_ignored_path, relative_to=self.get_project_root(), ) # TODO (maybe): not super efficient to walk through the files again and filter if glob patterns are provided