diff --git a/src/serena/project_server.py b/src/serena/project_server.py index 4deaa7c3..4e8a69c8 100644 --- a/src/serena/project_server.py +++ b/src/serena/project_server.py @@ -36,7 +36,7 @@ class QueryProjectRequest(BaseModel): class CallFacadeMethodRequest(BaseModel): """ - Request model for the /call_facade_method endpoint: the execution of a (read-only) REPL facade method + Request model for the /call_facade_method endpoint: the execution of a REPL facade method in the context of a project. """ @@ -166,14 +166,11 @@ class ProjectServer: """ Handles a /call_facade_method request by executing the facade method on the agent's REPL facades in the context of the specified project (see `_query_project` regarding the lock). - Only methods which use the project server and do not edit are admissible. """ project = self._get_project(req.project_name) with self._active_project_lock, self._agent.active_project_context(project): facade = self._agent.get_repl().entrypoint.get_facade_(req.facade_name) method = facade.get_method(req.method_name) - if not method.enabled or method.info.can_edit or not method.info.uses_project_server: - raise ValueError(f"Method '{req.facade_name}.{req.method_name}' cannot be executed via the project server") return self._agent.execute_task(lambda: method(*req.args, **req.kwargs)) def run(self) -> None: diff --git a/src/serena/repl/api/ext_api.py b/src/serena/repl/api/ext_api.py index 6554b098..eed18827 100644 --- a/src/serena/repl/api/ext_api.py +++ b/src/serena/repl/api/ext_api.py @@ -9,7 +9,7 @@ from typing import TYPE_CHECKING from serena.jetbrains.jetbrains_plugin_client import JetBrainsPluginClientManager from serena.tools import ListQueryableProjectsTool, QueryProjectTool -from ..external_project import ExternalProjectContext +from ..external_project import ExternalProjectExecution from ..facade import FacadeApi, facade_method from ..representable import JsonObject, JsonObjectRenderer @@ -24,14 +24,16 @@ class ExternalProjectContextManager: executed in the project's server. Contexts cannot be nested. """ - def __init__(self, agent: "SerenaAgent", project_name: str) -> None: + def __init__(self, agent: "SerenaAgent", project_name: str, read_only: bool) -> None: """ :param agent: the agent :param project_name: the name (or root path) of the registered external project + :param read_only: whether the context is read-only """ self._agent = agent self._project_name = project_name self._active_project_context = None + self._read_only = read_only def __enter__(self) -> None: entrypoint = self._agent.get_repl().entrypoint @@ -46,9 +48,8 @@ class ExternalProjectContextManager: self._active_project_context = self._agent.active_project_context(project) self._active_project_context.__enter__() - # switch the facades to the external project (remote execution of language server operations for the LSP backend) - remote_execution = self._agent.get_language_backend().is_lsp() - entrypoint.set_external_project_(ExternalProjectContext(registered_project.project_name, remote_execution)) + # switch the facades to the external project + entrypoint.set_external_project_(ExternalProjectExecution(registered_project.project_name, self._read_only, self._agent)) def __exit__(self, exc_type: type[BaseException] | None, exc_value: BaseException | None, traceback: TracebackType | None) -> None: self._agent.get_repl().entrypoint.set_external_project_(None) @@ -80,7 +81,7 @@ class ExternalProjectsApi(FacadeApi): return JsonObject(result, JsonObjectRenderer(self._agent, -1)) @facade_method(corresponding_tool=QueryProjectTool) - def project_context(self, project_name: str) -> ExternalProjectContextManager: + def read_project_context(self, project_name: str) -> ExternalProjectContextManager: """ Provides a context (for use in a `with` statement) within which all facades operate on the given external project instead of the active one, with read-only access. @@ -94,4 +95,21 @@ class ExternalProjectsApi(FacadeApi): :return: the context manager """ - return ExternalProjectContextManager(self._agent, project_name) + return ExternalProjectContextManager(self._agent, project_name, read_only=True) + + @facade_method(optional=True) + def project_context(self, project_name: str) -> ExternalProjectContextManager: + """ + Provides a context (for use in a `with` statement) within which all facades operate on the given external project + instead of the active one (read and write operations are possible). + + Example: + `with s.ext.project_context("other"): result = s.lsp.find_symbol("Foo")` + + Results obtained within the context can be used after it (they are self-contained). + + :param project_name: the name (or root path) of the project, as listed by `list_projects` + :return: the context manager + + """ + return ExternalProjectContextManager(self._agent, project_name, read_only=False) diff --git a/src/serena/repl/external_project.py b/src/serena/repl/external_project.py index e702d4ff..1e909b39 100644 --- a/src/serena/repl/external_project.py +++ b/src/serena/repl/external_project.py @@ -8,8 +8,11 @@ from typing import TYPE_CHECKING, Any if TYPE_CHECKING: from serena.project_server import ProjectServerClient + from ..agent import SerenaAgent + from .facade import FacadeMethod -class ExternalProjectContext: + +class ExternalProjectExecution: """ The context in which facade methods are executed while an external project is being queried: methods which use the project server (see `FacadeMethodInfo.uses_project_server`) are executed remotely @@ -17,26 +20,39 @@ class ExternalProjectContext: locally against the temporarily switched project. Editing methods are not permitted. """ - def __init__(self, project_name: str, remote_execution: bool) -> None: + def __init__(self, project_name: str, read_only: bool, agent: "SerenaAgent") -> None: """ :param project_name: the name of the external project - :param remote_execution: whether methods using the project server are to be executed remotely - (False for the JetBrains backend, where the IDE serves all projects) + :param read_only: whether the external project is to be treated as read-only (editing methods are not permitted) """ self.project_name = project_name - self._remote_execution = remote_execution self._client: ProjectServerClient | None = None + self._read_only = read_only + self._agent = agent - def executes_remotely(self, uses_project_server: bool) -> bool: + def is_called_remotely(self, method: "FacadeMethod") -> bool: """ - :param uses_project_server: whether the method in question uses the project server - :return: whether the method is to be executed remotely + :param method: the method to check + :return: whether the given method must be executed remotely """ - return self._remote_execution and uses_project_server + # Any method that uses the project server must be executed remotely, + # as does any edit operation when using the LSP backend (as edit operations indirectly + # use the language server via the CodeEditor abstraction) + return method.info.uses_project_server or (self._agent.get_language_backend().is_lsp() and method.info.can_edit) - def call(self, facade_name: str, method_name: str, args: tuple[Any, ...], kwargs: dict[str, Any]) -> Any: + def check_call_permission(self, method: "FacadeMethod") -> None: """ - Executes the given facade method in the external project's server. + Checks whether the given method is permitted to be called in the context of this external project execution. + Raises an exception if the method is not permitted. + + :param method: the facade method to check + """ + if self._read_only and method.info.can_edit: + raise PermissionError(f"Editing methods are not permitted in read-only external project execution: {method.qualified_name}") + + def call_remotely(self, facade_name: str, method_name: str, args: tuple[Any, ...], kwargs: dict[str, Any]) -> Any: + """ + Executes the given facade method remotely via the project server. :param facade_name: the facade's name :param method_name: the method's name diff --git a/src/serena/repl/facade.py b/src/serena/repl/facade.py index 2f198fa7..cfe53a7c 100644 --- a/src/serena/repl/facade.py +++ b/src/serena/repl/facade.py @@ -26,7 +26,7 @@ if TYPE_CHECKING: from serena.code_editor import CodeEditor from serena.tools import Tool - from .external_project import ExternalProjectContext + from .external_project import ExternalProjectExecution log = logging.getLogger(__name__) TCallable = TypeVar("TCallable", bound=Callable[..., Any]) @@ -253,7 +253,10 @@ class FacadeMethodInfo: uses_project_server: bool = False """ whether the method requires the project's language servers and must therefore be executed in the project server - when an external project is queried (see `ExternalProjectContext`) + when an external project is queried (see `ExternalProjectContext`). + Edit operations are always executed in the project server, regardless of this flag, since they implicitly + use the CodeEditor, which requires language servers when using the LSP backend. + Polymorphic edit operations therefore must not set this flag to True. """ corresponding_tool: "type[Tool] | None" = None """the classic tool offering the same functionality, if any""" @@ -285,7 +288,7 @@ def facade_method( :param beta: whether the method is in beta :param can_edit: whether the method can modify the codebase :param niche: whether the method is rarely needed (its documentation is then only summarised in the facade's description) - :param uses_project_server: whether the method must be executed in the project server when an external project is queried + :param uses_project_server: whether the method must be executed remotely in the project server when an external project is queried :param corresponding_tool: the classic tool offering the same functionality, if any :return: the decorator """ @@ -406,12 +409,11 @@ class FacadeMethod: return f"{self.facade_name}.{self.name}" def __call__(self, *args: Any, **kwargs: Any) -> Any: - external_project = self.parent.get_external_project_() - if external_project is not None: - if self.info.can_edit: - raise ValueError(f"{self.qualified_name} cannot be called while an external project is being queried (read-only access)") - if external_project.executes_remotely(self.info.uses_project_server): - return external_project.call(self.facade_name, self.name, args, kwargs) + external_project_execution = self.parent.get_external_project_() + if external_project_execution is not None: + external_project_execution.check_call_permission(self) + if external_project_execution.is_called_remotely(self): + return external_project_execution.call_remotely(self.facade_name, self.name, args, kwargs) return self._implementation(*args, **kwargs) def get_implementation_(self) -> Callable[..., Any]: @@ -579,13 +581,13 @@ class Facade: object.__setattr__(self, "_types", {t.name: t for t in types}) object.__setattr__(self, "_external_project", None) - def set_external_project_(self, external_project: "ExternalProjectContext | None") -> None: + def set_external_project_(self, external_project: "ExternalProjectExecution | None") -> None: """ :param external_project: the context of the external project being queried (None if the active project is used) """ object.__setattr__(self, "_external_project", external_project) - def get_external_project_(self) -> "ExternalProjectContext | None": + def get_external_project_(self) -> "ExternalProjectExecution | None": return self._external_project def _add_method(self, method: FacadeMethod) -> None: diff --git a/src/serena/repl/repl.py b/src/serena/repl/repl.py index c9c29fe1..9dc1b7b1 100644 --- a/src/serena/repl/repl.py +++ b/src/serena/repl/repl.py @@ -12,7 +12,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Any from ..session import SerenaSession -from .external_project import ExternalProjectContext +from .external_project import ExternalProjectExecution from .facade import ApiScope, Facade, FacadeMethod, ReferencedType from .representable import Representable @@ -96,7 +96,7 @@ class SerenaReplEntrypoint: return True return False - def set_external_project_(self, external_project: "ExternalProjectContext | None") -> None: + def set_external_project_(self, external_project: "ExternalProjectExecution | None") -> None: """ :param external_project: the context of the external project being queried by the currently executing code (None if the active project is used); propagated to all facades @@ -104,7 +104,7 @@ class SerenaReplEntrypoint: for facade in self._facades.values(): facade.set_external_project_(external_project) - def get_external_project_(self) -> "ExternalProjectContext | None": + def get_external_project_(self) -> "ExternalProjectExecution | None": external_projects = {facade.get_external_project_() for facade in self._facades.values()} return next(iter(external_projects)) if external_projects else None diff --git a/test/serena/test_external_projects.py b/test/serena/test_external_projects.py index bee10c4d..a2bb73d7 100644 --- a/test/serena/test_external_projects.py +++ b/test/serena/test_external_projects.py @@ -71,10 +71,6 @@ def test_facade_method_results_are_transferred_from_the_project_server(project_s assert result.symbols[0].body.startswith("def create_user") assert "create_user" in result.represent() - # editing methods are refused - with pytest.raises(ValueError, match="cannot be executed"): - client.call_facade_method("test_repo_python", "lsp", "rename_symbol", ["create_user", "test_repo/services.py", "x"], {}) - @pytest.mark.python @pytest.mark.skipif(not language_server_tests_enabled(LanguageServerId.PYTHON), reason="python tests are disabled in this environment") @@ -96,7 +92,7 @@ def test_external_project_context_in_repl( tool = agent.get_tool(SerenaReplTool) session_id = agent.create_session().session_id code = ( - 'with s.ext.project_context("test_repo_python"):\n' + 'with s.ext.read_project_context("test_repo_python"):\n' ' result = s.lsp.find_symbol("create_user")\n' "[s.name for s in result.symbols]" ) diff --git a/test/serena/test_repl_tool.py b/test/serena/test_repl_tool.py index 3be9dc42..f3f0a5f5 100644 --- a/test/serena/test_repl_tool.py +++ b/test/serena/test_repl_tool.py @@ -7,10 +7,10 @@ from unittest.mock import MagicMock import pytest -from serena.config.serena_config import ApiInclusionDefinition +from serena.config.serena_config import ApiInclusionDefinition, LanguageBackend from serena.repl.api.edit_api import EditApi from serena.repl.api.lsp_api import LspApi -from serena.repl.external_project import ExternalProjectContext +from serena.repl.external_project import ExternalProjectExecution from serena.repl.facade import ApiScope, Facade, FacadeApi, FacadeMethodInfo, facade_method from serena.repl.repl import SerenaRepl from serena.session import SerenaSession @@ -83,29 +83,51 @@ class TestReplExecution: overview = rebuilt_repl.execute("facades()", session) assert "s.edit" in overview and "s.lsp" not in overview - def test_external_project_dispatch(self) -> None: - class FakeExternalProject(ExternalProjectContext): + @pytest.mark.parametrize("backend", [LanguageBackend.LSP, LanguageBackend.JETBRAINS]) + @pytest.mark.parametrize("read_only", [True, False]) + def test_external_project_dispatch(self, backend: LanguageBackend, read_only: bool) -> None: + agent = MagicMock() + agent.get_language_backend.return_value = backend + + class FakeExternalProject(ExternalProjectExecution): def __init__(self) -> None: - super().__init__("other", remote_execution=True) + super().__init__("other", read_only=read_only, agent=agent) self.calls: list[tuple[str, str, tuple, dict]] = [] - def call(self, facade_name: str, method_name: str, args: tuple, kwargs: dict) -> str: + def call_remotely(self, facade_name: str, method_name: str, args: tuple, kwargs: dict) -> str: self.calls.append((facade_name, method_name, args, kwargs)) return "remote result" - facades = [Facade.from_api(LspApi(MagicMock()), ApiScope()), Facade.from_api(EditApi(MagicMock()), ApiScope())] + class LocalApi(FacadeApi): + @facade_method(can_edit=True) + def write(self, content: str) -> str: + return f"local result: {content}" + + # expose a server-backed read and a backend-dependent write + facades = [Facade.from_api(LspApi(agent), ApiScope()), Facade.from_api(LocalApi(agent, "local", "local operations"), ApiScope())] repl = SerenaRepl(facades, ApiScope()) external_project = FakeExternalProject() repl.entrypoint.set_external_project_(external_project) - # methods using the project server are executed remotely, editing methods are refused + # methods explicitly requiring the project server are executed remotely assert repl.execute('s.lsp.find_symbol("Foo", depth=1)') == "remote result" assert external_project.calls == [("lsp", "find_symbol", ("Foo",), {"depth": 1})] - assert "read-only" in repl.execute('s.edit.replace_content("a.py", "x", "y", "literal")') - # without remote execution (JetBrains backend), methods run locally - repl.entrypoint.set_external_project_(ExternalProjectContext("other", remote_execution=False)) - assert "remote result" not in repl.execute('s.info("lsp.find_symbol")') + # writes obey the context's access mode and use the selected backend + result = repl.execute('s.local.write("content")') + if read_only: + assert "PermissionError" in result and "read-only" in result + assert len(external_project.calls) == 1 + elif backend.is_lsp(): + assert result == "remote result" + assert external_project.calls[-1] == ("local", "write", ("content",), {}) + else: + assert result == "local result: content" + assert len(external_project.calls) == 1 + + # leaving the external execution context restores local execution + repl.entrypoint.set_external_project_(None) + assert repl.execute('s.local.write("restored")') == "local result: restored" def test_facade_discovery(self, repl: SerenaRepl) -> None: overview = repl.execute("s.info()")