diff --git a/.serena/memories/repl.md b/.serena/memories/repl.md index 88db9700..b7a6683e 100644 --- a/.serena/memories/repl.md +++ b/.serena/memories/repl.md @@ -83,7 +83,10 @@ expression is the result. No `return` (a top-level `return` yields a SyntaxError (a capability constraint, e.g. clients that handle the REPL badly), with the user's preference choosing among them. - `included_apis`/`excluded_apis` (references `facade` or `facade.method`) in global config, context, modes, project config; applied in that order via `ApiScope` (exclusions first, then inclusions; later definitions win). - Optional methods and all methods of an excluded facade must be included explicitly. +- Opt-in rule: methods of a facade that is not included (excluded, or optional without explicit inclusion) and + optional methods are enabled only if included explicitly; all other methods are enabled unless excluded. + Facades can be optional (`Facade.from_api(..., is_optional=True)`, e.g. `ext`, mirroring optional tools); + `Facade.is_enabled()` is derived: a facade is available iff it has at least one enabled method. - The REPL is rebuilt whenever the active tools are updated (mode switch, project activation). ## Availability policy diff --git a/src/serena/agent.py b/src/serena/agent.py index c0927467..1815160e 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -1344,7 +1344,7 @@ class SerenaAgent: Facade.from_api(EditApi(self), api_scope), Facade.from_api(MemoryApi(self), api_scope), Facade.from_api(ShellApi(self), api_scope), - Facade.from_api(ExternalProjectsApi(self), api_scope), + Facade.from_api(ExternalProjectsApi(self), api_scope, is_optional=True), ] if self._language_backend.is_lsp(): facades.append(Facade.from_api(LspApi(self), api_scope)) diff --git a/src/serena/repl/facade.py b/src/serena/repl/facade.py index cb66a012..7de3f172 100644 --- a/src/serena/repl/facade.py +++ b/src/serena/repl/facade.py @@ -478,17 +478,23 @@ class ApiScope: """ def __init__(self) -> None: - self.is_included = True + self._is_included: bool | None = None self.method_inclusions: set[str] = set() self.method_exclusions: set[str] = set() def exclude_facade(self) -> None: - self.is_included = False + self._is_included = False self.method_inclusions = set() self.method_exclusions = set() def include_facade(self) -> None: - self.is_included = True + self._is_included = True + + def is_facade_included(self, is_facade_optional: bool) -> bool: + if is_facade_optional: + return self._is_included is True + else: + return self._is_included is not False def exclude_method(self, method_name: str) -> None: self.method_inclusions.discard(method_name) @@ -537,23 +543,24 @@ class ApiScope: """ self._editing_excluded = True - def is_facade_enabled(self, facade_name: str) -> bool: - facade_scope = self._get_facade_scope(facade_name) - return facade_scope.is_included or len(facade_scope.method_inclusions) > 0 - - def is_method_enabled(self, facade_name: str, method_info: FacadeMethodInfo) -> bool: + def is_method_enabled(self, facade_name: str, method_info: FacadeMethodInfo, is_facade_optional: bool) -> bool: """ :param facade_name: the name of the facade :param method_info: the method's metadata - :return: whether the method is enabled: optional methods (and all methods of an excluded facade) must be - explicitly included, other methods are enabled unless explicitly excluded; if editing is excluded, - editing methods are always disabled + :param is_facade_optional: whether the facade is optional (disabled by default and must be enabled explicitly) + :return: whether the method is enabled: optional methods (and all methods of a facade which is not included, + i.e. an excluded facade or an optional facade that was not explicitly included) must be explicitly + included, other methods are enabled unless explicitly excluded; if editing is excluded, editing + methods are always disabled """ if self._editing_excluded and method_info.can_edit: return False facade_scope = self._get_facade_scope(facade_name) - if method_info.optional or not facade_scope.is_included: + # A method that would be disabled because the facade it is part of is not included + # or the method itself is optional must be explicitly included in order to be enabled. + if not facade_scope.is_facade_included(is_facade_optional) or method_info.optional: return method_info.name in facade_scope.method_inclusions + # A method that is not optional and whose facade is included is enabled unless it is explicitly excluded. else: return method_info.name not in facade_scope.method_exclusions @@ -585,12 +592,13 @@ class Facade: self._methods[method.name] = method @staticmethod - def from_api(api: FacadeApi, api_scope: ApiScope) -> "Facade": + def from_api(api: FacadeApi, api_scope: ApiScope, *, is_optional: bool = False) -> "Facade": """ Creates a facade wrapping the given implementation. :param api: the implementation; each of its methods decorated with `facade_method` becomes a facade method :param api_scope: API scope definition determining which methods are enabled + :param is_optional: whether the facade is optional (disabled by default and must be enabled explicitly) :return: the facade """ facade = Facade(api.get_name_(), api.get_description_(), api.get_referenced_types_()) @@ -598,11 +606,17 @@ class Facade: method_info = get_facade_method_info(member) if method_info is None: continue - is_enabled = api_scope.is_method_enabled(facade.name, method_info) + is_enabled = api_scope.is_method_enabled(facade.name, method_info, is_facade_optional=is_optional) facade._add_method(FacadeMethod(facade, member, method_info, enabled=is_enabled)) facade._discover_referenced_types() return facade + def is_enabled(self) -> bool: + """ + :return: whether the facade is enabled + """ + return len(self.get_enabled_methods()) > 0 + def _discover_referenced_types(self) -> None: """ Adds referenced types for all classes reachable (transitively) through the annotations of the facade's methods diff --git a/src/serena/repl/repl.py b/src/serena/repl/repl.py index 1e697738..c9c29fe1 100644 --- a/src/serena/repl/repl.py +++ b/src/serena/repl/repl.py @@ -41,14 +41,9 @@ class FacadeAvailabilityInfo: def __init__(self): self.facades: list[FacadeAvailabilityInfo.FacadeInfo] = [] - def add_facade(self, facade: Facade, is_enabled: bool): - def is_method_enabled(m: FacadeMethod) -> bool: - return is_enabled and m.enabled - - methods_info = [ - FacadeAvailabilityInfo.MethodInfo(name=method.name, is_enabled=is_method_enabled(method)) for method in facade.get_methods() - ] - self.facades.append(FacadeAvailabilityInfo.FacadeInfo(name=facade.name, is_enabled=is_enabled, methods=methods_info)) + def add_facade(self, facade: Facade): + methods_info = [FacadeAvailabilityInfo.MethodInfo(name=method.name, is_enabled=method.enabled) for method in facade.get_methods()] + self.facades.append(FacadeAvailabilityInfo.FacadeInfo(name=facade.name, is_enabled=facade.is_enabled(), methods=methods_info)) class SerenaReplEntrypoint: @@ -68,9 +63,8 @@ class SerenaReplEntrypoint: self._facade_availability_info = FacadeAvailabilityInfo() registered_facade_names = [] for facade in facades: - is_facade_enabled = api_scope.is_facade_enabled(facade.name) - self._facade_availability_info.add_facade(facade, is_facade_enabled) - if is_facade_enabled: + self._facade_availability_info.add_facade(facade) + if facade.is_enabled(): if facade.name in self._facades: raise ValueError(f"Duplicate facade name: {facade.name}") self._facades[facade.name] = facade diff --git a/src/serena/resources/config/modes/query-projects.yml b/src/serena/resources/config/modes/query-projects.yml index ed140de9..9ceea8fa 100644 --- a/src/serena/resources/config/modes/query-projects.yml +++ b/src/serena/resources/config/modes/query-projects.yml @@ -6,3 +6,5 @@ excluded_tools: [] included_optional_tools: - list_queryable_projects - query_project +included_apis: + - ext diff --git a/test/serena/test_external_projects.py b/test/serena/test_external_projects.py index a53b5ed4..bee10c4d 100644 --- a/test/serena/test_external_projects.py +++ b/test/serena/test_external_projects.py @@ -86,6 +86,9 @@ def test_external_project_context_in_repl( server, port = project_server monkeypatch.setattr(ProjectServer, "PORT", port) # let the REPL's external project context use the test server + # enable the optional "ext" facade + serena_config.included_apis = ["ext"] + # the querying agent has another project active and queries the python test project agent = SerenaAgent(project="test_repo_typescript", serena_config=serena_config) agent.execute_task(lambda: None) diff --git a/test/serena/test_repl_tool.py b/test/serena/test_repl_tool.py index f02cbbcb..3be9dc42 100644 --- a/test/serena/test_repl_tool.py +++ b/test/serena/test_repl_tool.py @@ -217,6 +217,22 @@ class TestFacade: scope.process(ApiInclusionDefinition(**kwargs)) return scope + def test_optional_facade_is_opt_in(self) -> None: + # an optional facade is disabled unless it is included explicitly + facade = Facade.from_api(self.DummyApi(MagicMock()), ApiScope(), is_optional=True) + assert not facade.is_enabled() + assert facade.enabled_method_names == [] + + # including the facade enables its non-optional methods + facade = Facade.from_api(self.DummyApi(MagicMock()), self._scope(included_apis=["dummy"]), is_optional=True) + assert facade.is_enabled() + assert set(facade.enabled_method_names) == {"add", "secret", "rarely"} + + # including a single method enables the facade with just that method + facade = Facade.from_api(self.DummyApi(MagicMock()), self._scope(included_apis=["dummy.add"]), is_optional=True) + assert facade.is_enabled() + assert facade.enabled_method_names == ["add"] + def test_api_scope_facade_exclusion_and_method_inclusion(self) -> None: # excluding the facade disables everything facade = Facade.from_api(self.DummyApi(MagicMock()), self._scope(excluded_apis=["dummy"]))