From 13a96851efa22d5ea25a1bcf920f8cf4d03ee29d Mon Sep 17 00:00:00 2001 From: CREDO23 Date: Thu, 4 Jun 2026 13:16:22 +0200 Subject: [PATCH] refactor(agents): move skills/, plugins/, plugin_loader to app/agents/shared (slice 7) - skills/ (builtin SKILL.md assets) has zero Python importers; it is read by filesystem path only. Moved the dir and restored skills_backends._default_builtin_root() to the clean parent.parent / "skills" / "builtin" form (undoing the transient path from 5c). - plugin_loader.py -> shared (frozen chat_deepagent uses it -> re-export shim). - plugins/ package -> shared (year_substituter rewired to shared.plugin_loader; docstring entry-point example updated to the shared dotted path). No shim needed (only a test imported it). Plugin discovery is via importlib entry points (group "surfsense.plugins"), not dotted-path import, and nothing is registered in pyproject, so the move does not affect runtime discovery. --- .../middleware/main_agent/plugins.py | 2 +- .../app/agents/new_chat/plugin_loader.py | 157 +---------------- .../shared/middleware/skills_backends.py | 12 +- .../app/agents/shared/plugin_loader.py | 158 ++++++++++++++++++ .../{new_chat => shared}/plugins/__init__.py | 0 .../plugins/year_substituter.py | 4 +- .../{new_chat => shared}/skills/__init__.py | 0 .../skills/builtin/__init__.py | 0 .../skills/builtin/email-drafting/SKILL.md | 0 .../skills/builtin/kb-research/SKILL.md | 0 .../skills/builtin/meeting-prep/SKILL.md | 0 .../skills/builtin/report-writing/SKILL.md | 0 .../skills/builtin/slack-summary/SKILL.md | 0 .../agents/new_chat/test_plugin_loader.py | 16 +- 14 files changed, 182 insertions(+), 167 deletions(-) create mode 100644 surfsense_backend/app/agents/shared/plugin_loader.py rename surfsense_backend/app/agents/{new_chat => shared}/plugins/__init__.py (100%) rename surfsense_backend/app/agents/{new_chat => shared}/plugins/year_substituter.py (95%) rename surfsense_backend/app/agents/{new_chat => shared}/skills/__init__.py (100%) rename surfsense_backend/app/agents/{new_chat => shared}/skills/builtin/__init__.py (100%) rename surfsense_backend/app/agents/{new_chat => shared}/skills/builtin/email-drafting/SKILL.md (100%) rename surfsense_backend/app/agents/{new_chat => shared}/skills/builtin/kb-research/SKILL.md (100%) rename surfsense_backend/app/agents/{new_chat => shared}/skills/builtin/meeting-prep/SKILL.md (100%) rename surfsense_backend/app/agents/{new_chat => shared}/skills/builtin/report-writing/SKILL.md (100%) rename surfsense_backend/app/agents/{new_chat => shared}/skills/builtin/slack-summary/SKILL.md (100%) diff --git a/surfsense_backend/app/agents/multi_agent_chat/middleware/main_agent/plugins.py b/surfsense_backend/app/agents/multi_agent_chat/middleware/main_agent/plugins.py index d3be13bfd..12e9ec24c 100644 --- a/surfsense_backend/app/agents/multi_agent_chat/middleware/main_agent/plugins.py +++ b/surfsense_backend/app/agents/multi_agent_chat/middleware/main_agent/plugins.py @@ -8,7 +8,7 @@ from typing import Any from langchain_core.language_models import BaseChatModel from app.agents.shared.feature_flags import AgentFeatureFlags -from app.agents.new_chat.plugin_loader import ( +from app.agents.shared.plugin_loader import ( PluginContext, load_allowed_plugin_names_from_env, load_plugin_middlewares, diff --git a/surfsense_backend/app/agents/new_chat/plugin_loader.py b/surfsense_backend/app/agents/new_chat/plugin_loader.py index c52620d40..332a1ec26 100644 --- a/surfsense_backend/app/agents/new_chat/plugin_loader.py +++ b/surfsense_backend/app/agents/new_chat/plugin_loader.py @@ -1,154 +1,15 @@ -"""Entry-point based plugin loader for SurfSense agent middleware. +"""Backward-compatible shim. -LangChain's :class:`AgentMiddleware` ABC already covers the practical -surface most plugins need (``before_agent`` / ``before_model`` / -``wrap_tool_call`` / their async counterparts), so a SurfSense-specific -plugin protocol would be redundant. We just need a way to discover and -admit third-party middleware safely. - -A plugin is therefore just an installable Python package that registers a -factory callable under the ``surfsense.plugins`` entry-point group: - -.. code-block:: toml - - # in a plugin package's pyproject.toml - [project.entry-points."surfsense.plugins"] - year_substituter = "my_plugin:make_middleware" - -The factory has the signature ``Callable[[PluginContext], AgentMiddleware]``. -It receives a small, sanitized :class:`PluginContext` with the IDs and the -LLM the plugin is allowed to talk to — and **never** raw secrets, DB -sessions, or other connectors. - -## Trust model - -Plugins are loaded **only if** their entry-point ``name`` appears in -``allowed_plugins`` (admin-controlled, sourced from -``global_llm_config.yaml`` or :func:`load_allowed_plugin_names_from_env`). -There is **no env-driven auto-load**. A plugin failure is logged and -isolated; it does not break agent construction. +Moved to ``app.agents.shared.plugin_loader``. Re-exported here for the frozen +single-agent stack (``chat_deepagent``) until that stack is retired. """ -from __future__ import annotations - -import logging -import os -from collections.abc import Iterable -from importlib.metadata import entry_points -from typing import TYPE_CHECKING - -from langchain.agents.middleware import AgentMiddleware - -if TYPE_CHECKING: # pragma: no cover - type-only - from langchain_core.language_models import BaseChatModel - - from app.db import ChatVisibility - - -logger = logging.getLogger(__name__) - - -PLUGIN_ENTRY_POINT_GROUP = "surfsense.plugins" - - -class PluginContext(dict): - """Sanitized DI bag handed to each plugin factory. - - Backed by ``dict`` so plugins can inspect the keys they care about - without coupling to a concrete dataclass shape. Required keys: - - * ``search_space_id`` (int) - * ``user_id`` (str | None) - * ``thread_visibility`` (:class:`app.db.ChatVisibility`) - * ``llm`` (:class:`langchain_core.language_models.BaseChatModel`) - - The context **never** carries DB sessions, raw secrets, or other - connectors. If a future plugin genuinely needs DB access, that - integration goes through a rate-limited service interface, not - through this bag. - """ - - @classmethod - def build( - cls, - *, - search_space_id: int, - user_id: str | None, - thread_visibility: ChatVisibility, - llm: BaseChatModel, - ) -> PluginContext: - return cls( - search_space_id=search_space_id, - user_id=user_id, - thread_visibility=thread_visibility, - llm=llm, - ) - - -def load_plugin_middlewares( - ctx: PluginContext, - allowed_plugin_names: Iterable[str], -) -> list[AgentMiddleware]: - """Discover, allowlist-filter, and instantiate plugin middleware. - - For each entry-point in :data:`PLUGIN_ENTRY_POINT_GROUP` whose name is - in ``allowed_plugin_names``, load the factory and call it with ``ctx``. - The factory's return value must be an :class:`AgentMiddleware` instance; - anything else is logged and skipped. - - Errors are isolated — a plugin that raises during ``ep.load()`` or - factory invocation is logged at ``ERROR`` and ignored. Agent - construction continues with whatever plugins did succeed. - """ - allowed = {name for name in allowed_plugin_names if name} - if not allowed: - return [] - - out: list[AgentMiddleware] = [] - try: - eps = entry_points(group=PLUGIN_ENTRY_POINT_GROUP) - except Exception: # pragma: no cover - defensive (entry_points is robust) - logger.exception("Failed to enumerate plugin entry points") - return [] - - for ep in eps: - if ep.name not in allowed: - logger.info("Skipping non-allowlisted plugin %s", ep.name) - continue - try: - factory = ep.load() - except Exception: - logger.exception("Failed to load plugin %s", ep.name) - continue - try: - mw = factory(ctx) - except Exception: - logger.exception("Plugin %s factory raised", ep.name) - continue - if not isinstance(mw, AgentMiddleware): - logger.warning( - "Plugin %s returned %s, expected AgentMiddleware; skipping", - ep.name, - type(mw).__name__, - ) - continue - out.append(mw) - logger.info("Loaded plugin %s as %s", ep.name, type(mw).__name__) - return out - - -def load_allowed_plugin_names_from_env() -> set[str]: - """Read ``SURFSENSE_ALLOWED_PLUGINS`` (comma-separated) into a set. - - Provided as a thin convenience for deployments that don't surface plugins - through ``global_llm_config.yaml`` yet. Whitespace is stripped and empty - entries are dropped. - """ - raw = os.environ.get("SURFSENSE_ALLOWED_PLUGINS", "").strip() - if not raw: - return set() - return {token.strip() for token in raw.split(",") if token.strip()} - +from app.agents.shared.plugin_loader import ( + PLUGIN_ENTRY_POINT_GROUP, + PluginContext, + load_allowed_plugin_names_from_env, + load_plugin_middlewares, +) __all__ = [ "PLUGIN_ENTRY_POINT_GROUP", diff --git a/surfsense_backend/app/agents/shared/middleware/skills_backends.py b/surfsense_backend/app/agents/shared/middleware/skills_backends.py index 091926627..76a1e7f49 100644 --- a/surfsense_backend/app/agents/shared/middleware/skills_backends.py +++ b/surfsense_backend/app/agents/shared/middleware/skills_backends.py @@ -16,7 +16,7 @@ prompt at agent build time, not edited at runtime. Two backends are provided: * :class:`BuiltinSkillsBackend` — disk-backed read of bundled skills from - ``app/agents/new_chat/skills/builtin/``. + ``app/agents/shared/skills/builtin/``. * :class:`SearchSpaceSkillsBackend` — a thin read-only wrapper over :class:`KBPostgresBackend` that filters notes under the privileged folder ``/documents/_skills/``. @@ -59,14 +59,10 @@ _MAX_SKILL_FILE_SIZE = 10 * 1024 * 1024 def _default_builtin_root() -> Path: """Return the absolute path to the bundled builtin skills directory. - The skill assets still live at ``app/agents/new_chat/skills/builtin/`` (the - ``skills/`` tree migrates to the shared kernel in a later slice). This module - now lives under ``app/agents/shared/middleware/``, so we walk up to - ``app/agents/`` and back into ``new_chat/skills/builtin``. Once skills move, - this becomes ``Path(__file__).resolve().parent.parent / "skills" / "builtin"``. + Located at ``app/agents/shared/skills/builtin/`` relative to this module + (this module lives at ``app/agents/shared/middleware/skills_backends.py``). """ - agents_dir = Path(__file__).resolve().parent.parent.parent - return (agents_dir / "new_chat" / "skills" / "builtin").resolve() + return (Path(__file__).resolve().parent.parent / "skills" / "builtin").resolve() class BuiltinSkillsBackend(BackendProtocol): diff --git a/surfsense_backend/app/agents/shared/plugin_loader.py b/surfsense_backend/app/agents/shared/plugin_loader.py new file mode 100644 index 000000000..c52620d40 --- /dev/null +++ b/surfsense_backend/app/agents/shared/plugin_loader.py @@ -0,0 +1,158 @@ +"""Entry-point based plugin loader for SurfSense agent middleware. + +LangChain's :class:`AgentMiddleware` ABC already covers the practical +surface most plugins need (``before_agent`` / ``before_model`` / +``wrap_tool_call`` / their async counterparts), so a SurfSense-specific +plugin protocol would be redundant. We just need a way to discover and +admit third-party middleware safely. + +A plugin is therefore just an installable Python package that registers a +factory callable under the ``surfsense.plugins`` entry-point group: + +.. code-block:: toml + + # in a plugin package's pyproject.toml + [project.entry-points."surfsense.plugins"] + year_substituter = "my_plugin:make_middleware" + +The factory has the signature ``Callable[[PluginContext], AgentMiddleware]``. +It receives a small, sanitized :class:`PluginContext` with the IDs and the +LLM the plugin is allowed to talk to — and **never** raw secrets, DB +sessions, or other connectors. + +## Trust model + +Plugins are loaded **only if** their entry-point ``name`` appears in +``allowed_plugins`` (admin-controlled, sourced from +``global_llm_config.yaml`` or :func:`load_allowed_plugin_names_from_env`). +There is **no env-driven auto-load**. A plugin failure is logged and +isolated; it does not break agent construction. +""" + +from __future__ import annotations + +import logging +import os +from collections.abc import Iterable +from importlib.metadata import entry_points +from typing import TYPE_CHECKING + +from langchain.agents.middleware import AgentMiddleware + +if TYPE_CHECKING: # pragma: no cover - type-only + from langchain_core.language_models import BaseChatModel + + from app.db import ChatVisibility + + +logger = logging.getLogger(__name__) + + +PLUGIN_ENTRY_POINT_GROUP = "surfsense.plugins" + + +class PluginContext(dict): + """Sanitized DI bag handed to each plugin factory. + + Backed by ``dict`` so plugins can inspect the keys they care about + without coupling to a concrete dataclass shape. Required keys: + + * ``search_space_id`` (int) + * ``user_id`` (str | None) + * ``thread_visibility`` (:class:`app.db.ChatVisibility`) + * ``llm`` (:class:`langchain_core.language_models.BaseChatModel`) + + The context **never** carries DB sessions, raw secrets, or other + connectors. If a future plugin genuinely needs DB access, that + integration goes through a rate-limited service interface, not + through this bag. + """ + + @classmethod + def build( + cls, + *, + search_space_id: int, + user_id: str | None, + thread_visibility: ChatVisibility, + llm: BaseChatModel, + ) -> PluginContext: + return cls( + search_space_id=search_space_id, + user_id=user_id, + thread_visibility=thread_visibility, + llm=llm, + ) + + +def load_plugin_middlewares( + ctx: PluginContext, + allowed_plugin_names: Iterable[str], +) -> list[AgentMiddleware]: + """Discover, allowlist-filter, and instantiate plugin middleware. + + For each entry-point in :data:`PLUGIN_ENTRY_POINT_GROUP` whose name is + in ``allowed_plugin_names``, load the factory and call it with ``ctx``. + The factory's return value must be an :class:`AgentMiddleware` instance; + anything else is logged and skipped. + + Errors are isolated — a plugin that raises during ``ep.load()`` or + factory invocation is logged at ``ERROR`` and ignored. Agent + construction continues with whatever plugins did succeed. + """ + allowed = {name for name in allowed_plugin_names if name} + if not allowed: + return [] + + out: list[AgentMiddleware] = [] + try: + eps = entry_points(group=PLUGIN_ENTRY_POINT_GROUP) + except Exception: # pragma: no cover - defensive (entry_points is robust) + logger.exception("Failed to enumerate plugin entry points") + return [] + + for ep in eps: + if ep.name not in allowed: + logger.info("Skipping non-allowlisted plugin %s", ep.name) + continue + try: + factory = ep.load() + except Exception: + logger.exception("Failed to load plugin %s", ep.name) + continue + try: + mw = factory(ctx) + except Exception: + logger.exception("Plugin %s factory raised", ep.name) + continue + if not isinstance(mw, AgentMiddleware): + logger.warning( + "Plugin %s returned %s, expected AgentMiddleware; skipping", + ep.name, + type(mw).__name__, + ) + continue + out.append(mw) + logger.info("Loaded plugin %s as %s", ep.name, type(mw).__name__) + return out + + +def load_allowed_plugin_names_from_env() -> set[str]: + """Read ``SURFSENSE_ALLOWED_PLUGINS`` (comma-separated) into a set. + + Provided as a thin convenience for deployments that don't surface plugins + through ``global_llm_config.yaml`` yet. Whitespace is stripped and empty + entries are dropped. + """ + raw = os.environ.get("SURFSENSE_ALLOWED_PLUGINS", "").strip() + if not raw: + return set() + return {token.strip() for token in raw.split(",") if token.strip()} + + +__all__ = [ + "PLUGIN_ENTRY_POINT_GROUP", + "PluginContext", + "load_allowed_plugin_names_from_env", + "load_plugin_middlewares", +] diff --git a/surfsense_backend/app/agents/new_chat/plugins/__init__.py b/surfsense_backend/app/agents/shared/plugins/__init__.py similarity index 100% rename from surfsense_backend/app/agents/new_chat/plugins/__init__.py rename to surfsense_backend/app/agents/shared/plugins/__init__.py diff --git a/surfsense_backend/app/agents/new_chat/plugins/year_substituter.py b/surfsense_backend/app/agents/shared/plugins/year_substituter.py similarity index 95% rename from surfsense_backend/app/agents/new_chat/plugins/year_substituter.py rename to surfsense_backend/app/agents/shared/plugins/year_substituter.py index 2b7781b90..c0095ddd7 100644 --- a/surfsense_backend/app/agents/new_chat/plugins/year_substituter.py +++ b/surfsense_backend/app/agents/shared/plugins/year_substituter.py @@ -17,7 +17,7 @@ Wire-up in ``pyproject.toml`` (illustrative; the in-repo plugin doesn't need this -- it's already on the import path):: [project.entry-points."surfsense.plugins"] - year_substituter = "app.agents.new_chat.plugins.year_substituter:make_middleware" + year_substituter = "app.agents.shared.plugins.year_substituter:make_middleware" """ from __future__ import annotations @@ -34,7 +34,7 @@ if TYPE_CHECKING: # pragma: no cover - type-only from langchain_core.messages import ToolMessage from langgraph.types import Command - from app.agents.new_chat.plugin_loader import PluginContext + from app.agents.shared.plugin_loader import PluginContext logger = logging.getLogger(__name__) diff --git a/surfsense_backend/app/agents/new_chat/skills/__init__.py b/surfsense_backend/app/agents/shared/skills/__init__.py similarity index 100% rename from surfsense_backend/app/agents/new_chat/skills/__init__.py rename to surfsense_backend/app/agents/shared/skills/__init__.py diff --git a/surfsense_backend/app/agents/new_chat/skills/builtin/__init__.py b/surfsense_backend/app/agents/shared/skills/builtin/__init__.py similarity index 100% rename from surfsense_backend/app/agents/new_chat/skills/builtin/__init__.py rename to surfsense_backend/app/agents/shared/skills/builtin/__init__.py diff --git a/surfsense_backend/app/agents/new_chat/skills/builtin/email-drafting/SKILL.md b/surfsense_backend/app/agents/shared/skills/builtin/email-drafting/SKILL.md similarity index 100% rename from surfsense_backend/app/agents/new_chat/skills/builtin/email-drafting/SKILL.md rename to surfsense_backend/app/agents/shared/skills/builtin/email-drafting/SKILL.md diff --git a/surfsense_backend/app/agents/new_chat/skills/builtin/kb-research/SKILL.md b/surfsense_backend/app/agents/shared/skills/builtin/kb-research/SKILL.md similarity index 100% rename from surfsense_backend/app/agents/new_chat/skills/builtin/kb-research/SKILL.md rename to surfsense_backend/app/agents/shared/skills/builtin/kb-research/SKILL.md diff --git a/surfsense_backend/app/agents/new_chat/skills/builtin/meeting-prep/SKILL.md b/surfsense_backend/app/agents/shared/skills/builtin/meeting-prep/SKILL.md similarity index 100% rename from surfsense_backend/app/agents/new_chat/skills/builtin/meeting-prep/SKILL.md rename to surfsense_backend/app/agents/shared/skills/builtin/meeting-prep/SKILL.md diff --git a/surfsense_backend/app/agents/new_chat/skills/builtin/report-writing/SKILL.md b/surfsense_backend/app/agents/shared/skills/builtin/report-writing/SKILL.md similarity index 100% rename from surfsense_backend/app/agents/new_chat/skills/builtin/report-writing/SKILL.md rename to surfsense_backend/app/agents/shared/skills/builtin/report-writing/SKILL.md diff --git a/surfsense_backend/app/agents/new_chat/skills/builtin/slack-summary/SKILL.md b/surfsense_backend/app/agents/shared/skills/builtin/slack-summary/SKILL.md similarity index 100% rename from surfsense_backend/app/agents/new_chat/skills/builtin/slack-summary/SKILL.md rename to surfsense_backend/app/agents/shared/skills/builtin/slack-summary/SKILL.md diff --git a/surfsense_backend/tests/unit/agents/new_chat/test_plugin_loader.py b/surfsense_backend/tests/unit/agents/new_chat/test_plugin_loader.py index 5dbf765a7..fa7ec223b 100644 --- a/surfsense_backend/tests/unit/agents/new_chat/test_plugin_loader.py +++ b/surfsense_backend/tests/unit/agents/new_chat/test_plugin_loader.py @@ -6,13 +6,13 @@ from unittest.mock import MagicMock, patch from langchain.agents.middleware import AgentMiddleware -from app.agents.new_chat.plugin_loader import ( +from app.agents.shared.plugin_loader import ( PLUGIN_ENTRY_POINT_GROUP, PluginContext, load_allowed_plugin_names_from_env, load_plugin_middlewares, ) -from app.agents.new_chat.plugins.year_substituter import ( +from app.agents.shared.plugins.year_substituter import ( _YearSubstituterMiddleware, make_middleware as year_substituter_factory, ) @@ -66,7 +66,7 @@ class TestPluginLoaderBasics: ep = _FakeEntryPoint("dangerous_plugin", factory) with patch( - "app.agents.new_chat.plugin_loader.entry_points", + "app.agents.shared.plugin_loader.entry_points", return_value=[ep], ): result = load_plugin_middlewares( @@ -78,7 +78,7 @@ class TestPluginLoaderBasics: def test_loads_allowlisted_plugin(self) -> None: ep = _FakeEntryPoint("year_substituter", year_substituter_factory) with patch( - "app.agents.new_chat.plugin_loader.entry_points", + "app.agents.shared.plugin_loader.entry_points", return_value=[ep], ): result = load_plugin_middlewares( @@ -95,7 +95,7 @@ class TestPluginLoaderIsolation: ep = _FakeEntryPoint("buggy", crashing_factory) with patch( - "app.agents.new_chat.plugin_loader.entry_points", + "app.agents.shared.plugin_loader.entry_points", return_value=[ep], ): result = load_plugin_middlewares(_ctx(), allowed_plugin_names={"buggy"}) @@ -107,7 +107,7 @@ class TestPluginLoaderIsolation: ep = _FakeEntryPoint("liar", bad_factory) with patch( - "app.agents.new_chat.plugin_loader.entry_points", + "app.agents.shared.plugin_loader.entry_points", return_value=[ep], ): result = load_plugin_middlewares(_ctx(), allowed_plugin_names={"liar"}) @@ -121,7 +121,7 @@ class TestPluginLoaderIsolation: raise ImportError("cannot import") with patch( - "app.agents.new_chat.plugin_loader.entry_points", + "app.agents.shared.plugin_loader.entry_points", return_value=[_BrokenEP()], ): result = load_plugin_middlewares(_ctx(), allowed_plugin_names={"broken"}) @@ -137,7 +137,7 @@ class TestPluginLoaderIsolation: _FakeEntryPoint("crashing", crashing_factory), _FakeEntryPoint("ok", year_substituter_factory), ] - with patch("app.agents.new_chat.plugin_loader.entry_points", return_value=eps): + with patch("app.agents.shared.plugin_loader.entry_points", return_value=eps): result = load_plugin_middlewares( _ctx(), allowed_plugin_names={"crashing", "ok"} )