mirror of
https://github.com/MODSetter/SurfSense.git
synced 2026-06-06 20:15:17 +02:00
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.
This commit is contained in:
parent
aab95b9130
commit
13a96851ef
14 changed files with 182 additions and 167 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
158
surfsense_backend/app/agents/shared/plugin_loader.py
Normal file
158
surfsense_backend/app/agents/shared/plugin_loader.py
Normal file
|
|
@ -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",
|
||||
]
|
||||
|
|
@ -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__)
|
||||
|
|
@ -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"}
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue