diff --git a/src/multilspy/language_server.py b/src/multilspy/language_server.py index afdefc51..00374aa4 100644 --- a/src/multilspy/language_server.py +++ b/src/multilspy/language_server.py @@ -105,7 +105,7 @@ class LanguageServer: return dirname.startswith('.') @classmethod - def create(cls, config: MultilspyConfig, logger: MultilspyLogger, repository_root_path: str, add_gitignore_content_to_config: bool = True) -> "LanguageServer": + def create(cls, config: MultilspyConfig, logger: MultilspyLogger, repository_root_path: str) -> "LanguageServer": """ Creates a language specific LanguageServer instance based on the given configuration, and appropriate settings for the programming language. @@ -115,30 +115,8 @@ class LanguageServer: :param repository_root_path: The root path of the repository. :param config: The Multilspy configuration. :param logger: The logger to use. - :param add_gitignore_content_to_config: whether to add the content of the .gitignore file (if any found) to the config, so that - the paths ignored there are also ignored by the language server - :return LanguageServer: A language specific LanguageServer instance. """ - config = copy(config) # prevent mutation - if add_gitignore_content_to_config: - gitignore_path = os.path.join(repository_root_path, ".gitignore") - if not os.path.exists(gitignore_path): - logger.log( - f"Should ignore all files in gitignore but no .gitignore found at {gitignore_path}. Skipping.", - logging.WARNING - ) - gitignore_file_content = None - else: - if config.gitignore_file_content is not None: - raise ValueError( - f"Asked to add gitignore content to the config for {repository_root_path=} but there already is a non-empty entry there:\n{config.gitignore_file_content=}" - ) - logger.log(f"Reading gitignore file from {gitignore_path}", logging.DEBUG) - with open(gitignore_path, "r", encoding="utf-8") as f: - gitignore_file_content = f.read() - config.gitignore_file_content = gitignore_file_content - if config.code_language == Language.PYTHON: from multilspy.language_servers.pyright_language_server.pyright_server import ( PyrightServer, @@ -270,12 +248,7 @@ class LanguageServer: # Normalize separators (pathspec expects forward slashes) pattern = pattern.replace(os.path.sep, '/') processed_patterns.append(pattern) - # Combine explicitly passed patterns with the content of the .gitignore file - self.logger.log(f"Processing gitignore content", logging.DEBUG) - if config.gitignore_file_content is not None: - for line in config.gitignore_file_content.splitlines(): - if not line.startswith('#') and line.strip() != '': - processed_patterns.append(line.strip()) + self.logger.log(f"Processing {len(processed_patterns)} ignored paths from the config", logging.DEBUG) # Create a pathspec matcher from the processed patterns self._ignore_spec = pathspec.PathSpec.from_lines( @@ -285,7 +258,7 @@ class LanguageServer: def get_ignore_spec(self) -> pathspec.PathSpec: """Returns the pathspec matcher for the paths that were configured to be ignored through - the multilspy config file and the .gitignore file. + the multilspy config. This is is a subset of the full language-specific ignore spec that determines which files are relevant for the language server. @@ -339,7 +312,7 @@ class LanguageServer: normalized_path = normalized_path + '/' # Use the pathspec matcher to check if the path matches any ignore pattern - if self._ignore_spec.match_file(normalized_path): + if self.get_ignore_spec().match_file(normalized_path): return True return False @@ -1697,7 +1670,7 @@ class SyncLanguageServer: It is used to communicate with Language Servers of different programming languages. """ - def __init__(self, language_server: LanguageServer, timeout: Optional[int] = None): + def __init__(self, language_server: LanguageServer, timeout: Optional[float] = None): """ :param language_server: the async language server being wrapped :param timeout: the timeout, in seconds, to use for requests to the language server. @@ -1714,8 +1687,8 @@ class SyncLanguageServer: @classmethod def create( - cls, config: MultilspyConfig, logger: MultilspyLogger, repository_root_path: str, add_gitignore_content_to_config=True, - timeout: Optional[int] = None + cls, config: MultilspyConfig, logger: MultilspyLogger, repository_root_path: str, + timeout: Optional[float] = None ) -> "SyncLanguageServer": """ Creates a language specific LanguageServer instance based on the given configuration, and appropriate settings for the programming language. @@ -1725,13 +1698,11 @@ class SyncLanguageServer: :param repository_root_path: The root path of the repository (must be absolute). :param config: The Multilspy configuration. :param logger: The logger to use. - :param add_gitignore_content_to_config: whether to add the content of the .gitignore file (if any found) to the config, so that - the paths ignored there are also ignored by the language server :param timeout: the timeout, in seconds, to use for requests; if None, use no timeout :return SyncLanguageServer: A language specific LanguageServer instance. """ - return SyncLanguageServer(LanguageServer.create(config, logger, repository_root_path, add_gitignore_content_to_config=add_gitignore_content_to_config), timeout=timeout) + return SyncLanguageServer(LanguageServer.create(config, logger, repository_root_path), timeout=timeout) @property def repository_root_path(self) -> str: @@ -2313,7 +2284,7 @@ class SyncLanguageServer: def get_ignore_spec(self) -> pathspec.PathSpec: """Returns the pathspec matcher for the paths that were configured to be ignored through - the multilspy config file and the .gitignore file. + the multilspy config. This is is a subset of the full language-specific ignore spec that determines which files are relevant for the language server. diff --git a/src/multilspy/multilspy_config.py b/src/multilspy/multilspy_config.py index 2b4f7ca4..c1d77ae0 100644 --- a/src/multilspy/multilspy_config.py +++ b/src/multilspy/multilspy_config.py @@ -82,8 +82,6 @@ class MultilspyConfig: start_independent_lsp_process: bool = True ignored_paths: list[str] = field(default_factory=list) """Paths, dirs or glob-like patterns. The matching will follow the same logic as for .gitignore entries""" - gitignore_file_content: str | None = None - """Optional content of the gitignore file. If passed, will be used in addition to the explicitly passed ignored_paths for deciding which paths to ignore.""" @classmethod def from_dict(cls, env: dict): diff --git a/src/serena/agent.py b/src/serena/agent.py index 9e11d843..0795e83a 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -25,6 +25,7 @@ from typing import TYPE_CHECKING, Any, Literal, Self, TypeVar, Union, cast import yaml from overrides import override +from pathspec import PathSpec from ruamel.yaml.comments import CommentedMap from sensai.util import logging from sensai.util.logging import FallbackHandler @@ -41,7 +42,7 @@ from serena.dashboard import MemoryLogHandler, SerenaDashboardAPI from serena.prompt_factory import PromptFactory, SerenaPromptFactory from serena.symbol import SymbolManager from serena.text_utils import search_files -from serena.util.file_system import scan_directory +from serena.util.file_system import GitignoreParser, match_path, scan_directory from serena.util.general import load_yaml, save_yaml from serena.util.inspection import determine_programming_language_composition, iter_subclasses from serena.util.shell import execute_shell_command @@ -583,6 +584,8 @@ class SerenaAgent: self.symbol_manager: SymbolManager | None = None self.memories_manager: MemoriesManager | None = None self.lines_read: LinesRead | None = None + self.ignore_spec: PathSpec # not set to None to avoid assert statements + """Ignore spec, extracted from the project's gitignore files and the explicitly configured ignored paths.""" # Apply context and mode tool configurations if context is None: @@ -615,6 +618,46 @@ 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) + + 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 + + return match_path(str(relative_path), self.ignore_spec) + + 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: all tool instances, including the non-active ones. For MCP clients, we need to expose them all since typical @@ -685,6 +728,7 @@ class SerenaAgent: # start the language server self.reset_language_server() assert self.language_server is not None + self.ignore_spec = self.language_server.get_ignore_spec() # initialize project-specific instances self.symbol_manager = SymbolManager(self.language_server, self) @@ -805,9 +849,21 @@ class SerenaAgent: # instantiate and start the language server assert self._active_project is not None + ignored_paths = self._active_project.project_config.ignored_paths + if len(ignored_paths) > 0: + log.info(f"Using {len(ignored_paths)} ignored paths from the explicit project configuration.") + log.debug(f"Ignored paths: {ignored_paths}") + if self._active_project.project_config.ignore_all_files_in_gitignore: + log.info(f"Parsing all gitignore files in {self._active_project.project_root}") + gitignore_parser = GitignoreParser(self._active_project.project_root) + log.info(f"Found {len(gitignore_parser.get_ignore_specs())} gitignore files.") + for spec in gitignore_parser.get_ignore_specs(): + log.debug(f"Adding {len(spec.patterns)} patterns from {spec.file_path} to the ignored paths.") + ignored_paths.extend(spec.patterns) + log.debug(f"Using {len(ignored_paths)} ignored paths in total.") multilspy_config = MultilspyConfig( code_language=self._active_project.project_config.language, - ignored_paths=self._active_project.project_config.ignored_paths, + ignored_paths=ignored_paths, trace_lsp_communication=self.serena_config.trace_lsp_communication, ) ls_logger = MultilspyLogger(log_level=self.serena_config.log_level) @@ -816,7 +872,6 @@ class SerenaAgent: multilspy_config, ls_logger, self._active_project.project_root, - add_gitignore_content_to_config=self._active_project.project_config.ignore_all_files_in_gitignore, ) self.language_server.start() if not self.language_server.is_running(): @@ -1061,6 +1116,8 @@ 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) + result = self.language_server.retrieve_full_file_content(relative_path) result_lines = result.splitlines() if end_line is None: @@ -1093,11 +1150,17 @@ class CreateTextFileTool(Tool, ToolMarkerCanEdit): :param content: the (utf-8-encoded) content to write to the file :return: a message indicating success or failure """ - absolute_path = os.path.join(self.get_project_root(), relative_path) - os.makedirs(os.path.dirname(absolute_path), exist_ok=True) - with open(absolute_path, "w", encoding="utf-8") as f: - f.write(content) - return f"File created: {relative_path}" + self.agent.validate_relative_path(relative_path) + + abs_path = (Path(self.get_project_root()) / relative_path).resolve() + will_overwrite_existing = abs_path.exists() + + abs_path.parent.mkdir(parents=True, exist_ok=True) + abs_path.write_text(content, encoding="utf-8") + answer = f"File created: {relative_path}." + if will_overwrite_existing: + answer += " Overwrote existing file." + return answer class ListDirTool(Tool): @@ -1107,7 +1170,7 @@ class ListDirTool(Tool): def apply(self, relative_path: str, recursive: bool, max_answer_chars: int = _DEFAULT_MAX_ANSWER_LENGTH) -> str: """ - Lists files and directories in the given directory (optionally with recursion). + Lists all non-gitignored files and directories in the given directory (optionally with recursion). :param relative_path: the relative path to the directory to list; pass "." to scan the project root :param recursive: whether to scan subdirectories recursively @@ -1116,17 +1179,14 @@ class ListDirTool(Tool): required for the task. :return: a JSON object with the names of directories and files within the given directory """ - - def is_ignored_path(abs_path: str) -> bool: - rel_path = os.path.relpath(abs_path, self.get_project_root()) - return self.language_server.is_ignored_path(rel_path, ignore_unsupported_files=False) + self.agent.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=is_ignored_path, - is_ignored_file=is_ignored_path, + is_ignored_dir=self.agent.path_is_gitignored, + is_ignored_file=self.agent.path_is_gitignored, ) result = json.dumps({"dirs": dirs, "files": files}) @@ -1140,31 +1200,27 @@ class FindFileTool(Tool): def apply(self, file_mask: str, relative_path: str) -> str: """ - Finds files matching the given file mask within the given relative path + Finds non-gitignored files matching the given file mask within the given relative path :param file_mask: the filename or file mask (using the wildcards * or ?) to search for :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) - def is_ignored_path(abs_path: str) -> bool: - rel_path = os.path.relpath(abs_path, self.get_project_root()) - return self.language_server.is_ignored_path(rel_path, ignore_unsupported_files=False) + 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 is_ignored_path(abs_path): + if self.agent.path_is_gitignored(abs_path): return True filename = os.path.basename(abs_path) - is_ignored = not fnmatch(filename, file_mask) - if not is_ignored: - is_ignored = not fnmatch(filename, file_mask) - return is_ignored + return not fnmatch(filename, file_mask) dirs, files = scan_directory( - os.path.join(self.get_project_root(), relative_path), - relative_to=self.get_project_root(), + path=dir_to_scan, recursive=True, - is_ignored_dir=is_ignored_path, + is_ignored_dir=self.agent.path_is_gitignored, is_ignored_file=is_ignored_file, ) @@ -1512,6 +1568,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) 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) @@ -1833,16 +1890,16 @@ class SearchForPatternTool(Tool): else: # we walk through all files in the project starting from the root files_to_search = [] - ignore_spec = self.language_server.get_ignore_spec() for root, dirs, files in os.walk(self.get_project_root()): # Don't go into directories that are ignored by modifying dirs inplace # Explanation for the + "/" part: # pathspec can't handle the matching of directories if they don't end with a slash! # see https://github.com/cpburnz/python-pathspec/issues/89 - dirs[:] = [d for d in dirs if not ignore_spec.match_file(d + "/")] + dirs[:] = [d for d in dirs if not self.agent.path_is_gitignored(os.path.join(root, d))] for file in files: - if not ignore_spec.match_file(os.path.join(root, file)): - files_to_search.append(os.path.join(root, file)) + file_path = os.path.join(root, file) + if not self.agent.path_is_gitignored(file_path): + files_to_search.append(file_path) # TODO (maybe): not super efficient to walk through the files again and filter if glob patterns are provided # but it probably never matters and this version required no further refactoring matches = search_files( diff --git a/src/serena/util/file_system.py b/src/serena/util/file_system.py index e7f773af..9ded41da 100644 --- a/src/serena/util/file_system.py +++ b/src/serena/util/file_system.py @@ -78,17 +78,14 @@ def find_all_non_ignored_files(repo_root: str) -> list[str]: @dataclass class GitignoreSpec: - """ - Represents a single gitignore file and its parsed patterns. - - :param file_path: Path to the gitignore file - :param patterns: List of adjusted patterns from the gitignore file - :param pathspec: Compiled PathSpec object for pattern matching - """ - file_path: str + """Path to the gitignore file.""" patterns: list[str] = field(default_factory=list) + """List of patterns from the gitignore file. + The patterns are adjusted based on the gitignore file location. + """ pathspec: PathSpec = field(init=False) + """Compiled PathSpec object for pattern matching.""" def __post_init__(self) -> None: """Initialize the PathSpec from patterns.""" @@ -276,3 +273,14 @@ class GitignoreParser: """Reload all gitignore files from the repository.""" self.ignore_specs.clear() self._load_gitignore_files() + + +def match_path(path: str, path_spec: PathSpec) -> bool: + path = os.path.abspath(path) + normalized_path = str(path).replace(os.path.sep, "/") + + # pathspec can't handle the matching of directories if they don't end with a slash! + # see https://github.com/cpburnz/python-pathspec/issues/89 + if os.path.isdir(normalized_path) and not normalized_path.endswith("/"): + normalized_path = normalized_path + "/" + return path_spec.match_file(normalized_path)