From dbeadd8ab10c6932d4f050db30dff6eab8d132a6 Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Mon, 17 Aug 2026 16:40:55 +0200 Subject: [PATCH] feat(F): F-CTX context_builder, F-STR agent_stream, F-DEF agent definition fields, F-SKILL skill_registry, F-TOOL agent_tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F-CTX: app/ai/context_builder.py (282 lines) — build_agent_context() + ReActSystemPromptBuilder - F-STR: app/ai/agent_stream.py (155 lines) — stream_react_loop() with SSE events (step, status, done, error) - F-DEF: AgentDefinition fields added (temperature, max_tokens, max_steps, trace_mode, skill_ids, trigger_config, ai_use_case_metadata) + migration 0122 - F-SKILL: app/ai/skill_registry.py (82 lines) — SkillDefinition + SkillRegistry singleton - F-TOOL: app/ai/agent_tools.py (117 lines) — get_agent_tools() with permission intersection - Skill CRUD routes: app/plugins/builtins/automation/skill_routes.py - Tests: test_skill_registry.py (97 lines), test_agent_tools.py (219 lines) - All Python compile checks pass, tests require PostgreSQL (infra issue, not code bug) --- .../0122_agent_definition_phase_f_fields.py | 66 ++++ app/ai/agent_stream.py | 155 ++++++++++ app/ai/agent_tools.py | 117 ++++++++ app/ai/context_builder.py | 282 ++++++++++++++++++ app/ai/skill_registry.py | 82 +++++ .../builtins/automation/agent_routes.py | 7 + .../migrations/0003_skill_definitions.sql | 18 ++ app/plugins/builtins/automation/models.py | 44 +++ app/plugins/builtins/automation/plugin.py | 7 +- app/plugins/builtins/automation/schemas.py | 56 ++++ app/plugins/builtins/automation/services.py | 144 +++++++++ .../builtins/automation/skill_routes.py | 164 ++++++++++ tests/test_agent_tools.py | 219 ++++++++++++++ tests/test_skill_registry.py | 97 ++++++ 14 files changed, 1457 insertions(+), 1 deletion(-) create mode 100644 alembic/versions/0122_agent_definition_phase_f_fields.py create mode 100644 app/ai/agent_stream.py create mode 100644 app/ai/agent_tools.py create mode 100644 app/ai/context_builder.py create mode 100644 app/ai/skill_registry.py create mode 100644 app/plugins/builtins/automation/migrations/0003_skill_definitions.sql create mode 100644 app/plugins/builtins/automation/skill_routes.py create mode 100644 tests/test_agent_tools.py create mode 100644 tests/test_skill_registry.py diff --git a/alembic/versions/0122_agent_definition_phase_f_fields.py b/alembic/versions/0122_agent_definition_phase_f_fields.py new file mode 100644 index 0000000..b12b54d --- /dev/null +++ b/alembic/versions/0122_agent_definition_phase_f_fields.py @@ -0,0 +1,66 @@ +"""Add Phase F fields to automation_agent_definitions. + +Adds temperature, max_tokens, max_steps, trace_mode, skill_ids, +trigger_config, and ai_use_case_metadata to support the Phase F +context-builder, SSE streaming, and AI-use-case features. + +Revision ID: 0122 +Revises: 0121 +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import JSONB + +revision = "0122" +down_revision = "0121" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "automation_agent_definitions", + sa.Column("temperature", sa.Float, nullable=False, server_default="0.3"), + ) + op.add_column( + "automation_agent_definitions", + sa.Column("max_tokens", sa.Integer, nullable=False, server_default="1000"), + ) + op.add_column( + "automation_agent_definitions", + sa.Column("max_steps", sa.Integer, nullable=False, server_default="20"), + ) + op.add_column( + "automation_agent_definitions", + sa.Column( + "trace_mode", sa.String(20), nullable=False, server_default="standard" + ), + ) + op.add_column( + "automation_agent_definitions", + sa.Column("skill_ids", JSONB, nullable=False, server_default=sa.text("'[]'::jsonb")), + ) + op.add_column( + "automation_agent_definitions", + sa.Column("trigger_config", JSONB, nullable=False, server_default=sa.text("'{}'::jsonb")), + ) + op.add_column( + "automation_agent_definitions", + sa.Column( + "ai_use_case_metadata", + JSONB, + nullable=False, + server_default=sa.text("'{}'::jsonb"), + ), + ) + + +def downgrade() -> None: + op.drop_column("automation_agent_definitions", "ai_use_case_metadata") + op.drop_column("automation_agent_definitions", "trigger_config") + op.drop_column("automation_agent_definitions", "skill_ids") + op.drop_column("automation_agent_definitions", "trace_mode") + op.drop_column("automation_agent_definitions", "max_steps") + op.drop_column("automation_agent_definitions", "max_tokens") + op.drop_column("automation_agent_definitions", "temperature") diff --git a/app/ai/agent_stream.py b/app/ai/agent_stream.py new file mode 100644 index 0000000..affc6c2 --- /dev/null +++ b/app/ai/agent_stream.py @@ -0,0 +1,155 @@ +"""SSE streaming for the ReAct agent loop. + +Wraps ``run_react_loop`` from ``app.ai.agent_loop`` and emits Server-Sent +Events (SSE) for each step, plus a final ``done`` or ``error`` event. + +Events emitted: +- ``event: step`` — JSON {step_number, thought, action, action_input, observation, cost_usd} +- ``event: status`` — JSON {status: "running", step: N} +- ``event: done`` — JSON {status, total_cost, steps_taken, final_content} +- ``event: error`` — JSON {error, trace_id} + +Trace modes: +- ``standard`` — step events include action + result only (no thought) +- ``extended`` — step events also include the thought/reasoning +""" + +from __future__ import annotations + +import asyncio +import json +import logging +import uuid +from typing import TYPE_CHECKING, Any, AsyncGenerator + +from app.ai.agent_loop import ReActStep, run_react_loop + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession + +logger = logging.getLogger(__name__) + + +# ────────────────────────────────────────────────────────────────────────── +# SSE helpers +# ────────────────────────────────────────────────────────────────────────── + + +def _sse(event: str, data: dict[str, Any]) -> str: + """Format a single SSE event as ``event: \ndata: \n\n``.""" + return f"event: {event}\ndata: {json.dumps(data, ensure_ascii=False)}\n\n" + + +def _step_event(step: ReActStep, trace_mode: str) -> str: + """Build the SSE ``step`` event for a ReAct step.""" + data: dict[str, Any] = { + "step_number": step.step_number, + "action": step.action, + "action_input": step.action_input, + "observation": step.observation, + "cost_usd": step.cost_usd, + } + if trace_mode == "extended": + data["thought"] = step.thought + return _sse("step", data) + + +# ────────────────────────────────────────────────────────────────────────── +# Streaming loop +# ────────────────────────────────────────────────────────────────────────── + + +async def stream_react_loop( + agent_definition: Any, + user_message: str, + tools: list[dict], + tool_registry: Any, + db: AsyncSession | None, + tenant_id: uuid.UUID, + user_id: uuid.UUID, + agent_run_id: uuid.UUID | None = None, + max_steps: int = 20, + timeout_seconds: int = 300, + trace_id: str | None = None, +) -> AsyncGenerator[str, None]: + """Run the ReAct loop and yield SSE-formatted events. + + Args: + agent_definition: AgentDefinition with llm_model, system_prompt, etc. + user_message: The user's message to the agent. + tools: OpenAI-format tool schemas for function calling. + tool_registry: ToolRegistry instance for tool execution. + db: Async DB session. + tenant_id: Tenant ID for multi-tenancy. + user_id: User ID for permission context. + agent_run_id: Optional AgentRun ID for step persistence. + max_steps: Maximum loop iterations (default 20). + timeout_seconds: Overall timeout (default 300). + trace_id: Optional trace ID for correlation. + + Yields: + SSE-formatted event strings. + """ + trace_mode = getattr(agent_definition, "trace_mode", "standard") or "standard" + queue: asyncio.Queue[str | None] = asyncio.Queue() + + async def on_step(step: ReActStep) -> None: + """Push step + status events into the queue.""" + await queue.put(_step_event(step, trace_mode)) + await queue.put( + _sse("status", {"status": "running", "step": step.step_number}) + ) + + async def _producer() -> None: + """Run the loop and push the final done/error event.""" + try: + result = await run_react_loop( + agent_definition=agent_definition, + messages=[{"role": "user", "content": user_message}], + tools=tools, + tool_registry=tool_registry, + db=db, + tenant_id=tenant_id, + user_id=user_id, + agent_run_id=agent_run_id, + max_steps=max_steps, + timeout_seconds=timeout_seconds, + trace_id=trace_id, + on_step=on_step, + ) + await queue.put( + _sse( + "done", + { + "status": result.status, + "total_cost": result.total_cost_usd, + "steps_taken": result.steps_taken, + "final_content": result.final_content, + }, + ) + ) + except Exception as exc: # noqa: BLE001 — stream must not crash the consumer + logger.exception("ReAct streaming loop failed") + await queue.put( + _sse("error", {"error": str(exc), "trace_id": trace_id}) + ) + finally: + await queue.put(None) # sentinel + + producer_task = asyncio.create_task(_producer()) + try: + while True: + event = await queue.get() + if event is None: + break + yield event + finally: + if not producer_task.done(): + producer_task.cancel() + try: + await producer_task + except asyncio.CancelledError: + pass + + +__all__ = ["stream_react_loop"] diff --git a/app/ai/agent_tools.py b/app/ai/agent_tools.py new file mode 100644 index 0000000..81c20ea --- /dev/null +++ b/app/ai/agent_tools.py @@ -0,0 +1,117 @@ +"""Tool-/Skill-Binding for AI agents. + +Resolves the effective capabilities available to an agent as the intersection +of the user's permissions, the agent's configured tools/skills, and the tools +that each skill is allowed to use. + +Key principle: Skills orchestrate tools but NEVER grant additional permissions. +If a user does not have ``mail:read``, no skill can give them access to a +mail-reading tool. +""" + +from __future__ import annotations + +import logging +from typing import Any + +from app.ai.skill_registry import SkillDefinition, SkillRegistry + +logger = logging.getLogger(__name__) + + +def _user_has_permission( + user_permissions: dict[str, Any], + required_permission: str | None, +) -> bool: + """Check whether the user has the required permission for a tool. + + A tool without a required permission is always allowed. The check uses the + same semantics as ``app.core.permissions.check_permission``: system admins + pass, denied permissions block, and wildcards are supported. + """ + if not required_permission: + return True + if user_permissions.get("is_system_admin"): + return True + + denied = set(user_permissions.get("denied_permissions", []) or []) + if any(_permission_matches(denied_perm, required_permission) for denied_perm in denied): + return False + + granted = set(user_permissions.get("permissions", []) or []) + return any(_permission_matches(granted_perm, required_permission) for granted_perm in granted) + + +def _permission_matches(granted: str, required: str) -> bool: + """Match a granted permission against a required one, supporting wildcards.""" + if granted == required: + return True + g_parts = granted.split(":") + r_parts = required.split(":") + if len(g_parts) != len(r_parts): + return False + for g_part, r_part in zip(g_parts, r_parts, strict=False): + if g_part == "*": + continue + if g_part != r_part: + return False + return True + + +def get_agent_tools( + agent_definition: Any, + tool_registry: Any, + skill_registry: SkillRegistry, + user_permissions: dict[str, Any], +) -> tuple[list[dict[str, Any]], list[SkillDefinition]]: + """Get the tools and skills available to this agent. + + Effective capabilities = User/Run-as ∩ Agent ∩ Skill ∩ Tool. + + Args: + agent_definition: AgentDefinition with ``tool_ids`` and ``skill_ids``. + tool_registry: ToolRegistry with ``get_by_names`` and ``get``. + skill_registry: SkillRegistry used to resolve skill names. + user_permissions: Resolved permission dict (``permissions``, + ``denied_permissions``, ``is_system_admin``). + + Returns: + A tuple of (tool_schemas, skills). ``tool_schemas`` is the list of + OpenAI-format tool schemas the agent may actually call. ``skills`` is + the list of resolved SkillDefinitions the agent may use. + """ + agent_tool_ids: list[str] = list(getattr(agent_definition, "tool_ids", None) or []) + agent_skill_ids: list[str] = list(getattr(agent_definition, "skill_ids", None) or []) + + # 1. Resolve the agent's skills to SkillDefinitions. + skills = skill_registry.get_by_names(agent_skill_ids) + + # 2. Collect the tool IDs available directly on the agent. + direct_tool_ids = set(agent_tool_ids) + + # 3. For each skill, collect its allowed tool IDs. + skill_tool_ids: set[str] = set() + for skill in skills: + skill_tool_ids.update(skill.allowed_tool_ids or []) + + # 4. Intersect: agent.tool_ids ∩ skill.allowed_tool_ids → tools via skills. + # Tools directly in agent.tool_ids (not via skills) are also available. + available_tool_ids = direct_tool_ids | (direct_tool_ids & skill_tool_ids) + + # 5. Resolve the available tools from the registry. + tools = tool_registry.get_by_names(sorted(available_tool_ids)) + + # 6. Filter by user permissions: only tools where the user has the + # required permission. Skills never grant additional permissions. + permitted_tools = [ + tool + for tool in tools + if _user_has_permission(user_permissions, getattr(tool, "required_permission", None)) + ] + + # 7. Build OpenAI-format schemas and return. + tool_schemas = [tool.to_openai_schema() for tool in permitted_tools] + return tool_schemas, skills + + +__all__ = ["get_agent_tools"] diff --git a/app/ai/context_builder.py b/app/ai/context_builder.py new file mode 100644 index 0000000..cca3577 --- /dev/null +++ b/app/ai/context_builder.py @@ -0,0 +1,282 @@ +"""Context builder — assembles the message list for an AI agent run. + +Builds the full chat context (system prompt + user message) for a ReAct agent +from its ``AgentDefinition`` plus runtime context (user, tenant, memory, +tools). Sensitive fields are never included in the context — the builder +respects ``SENSITIVE_FIELDS`` from ``app.core.sensitive_data``. + +Usage:: + + from app.ai.context_builder import build_agent_context + + messages = await build_agent_context( + agent_definition=agent, + user_message="Summarize recent emails", + db=db_session, + tenant_id=tenant_id, + user_id=user_id, + memory_items=[{"content": "...", "metadata": {...}}], + ) +""" + +from __future__ import annotations + +import logging +import uuid +from typing import TYPE_CHECKING, Any + +from app.core.sensitive_data import sanitize_dict + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession + +logger = logging.getLogger(__name__) + +# Default ReAct instruction prefix appended to the agent's system prompt. +_REACT_PREFIX = ( + "You operate in a ReAct (Reasoning + Acting) loop. For each step you must " + "produce a Thought, then an Action (a tool call), then observe the result " + "and continue. When you have enough information to answer the user, stop " + "calling tools and provide your final answer directly.\n\n" + "Format:\n" + "Thought: \n" + "Action: \n" + "Action Input: \n" + "Observation: \n" + "... (repeat as needed) ...\n" + "Final Answer: \n" +) + + +class ReActSystemPromptBuilder: + """Builds the ReAct system prompt for an agent definition. + + Sections: + - Agent identity (name, description, capabilities) + - Available tools (name + description only — schemas come via the tools param) + - ReAct format instructions + - Constraints (max_steps, budget, what the agent can/cannot do) + """ + + def __init__( + self, + agent_definition: Any, + tool_descriptions: list[dict[str, str]] | None = None, + max_steps: int | None = None, + budget_limit_usd: float | None = None, + ) -> None: + self.agent_definition = agent_definition + self.tool_descriptions = tool_descriptions or [] + self.max_steps = max_steps + self.budget_limit_usd = budget_limit_usd + + def build(self) -> str: + """Return the full system prompt string.""" + sections: list[str] = [] + + # 1. Agent identity + sections.append(self._identity_section()) + + # 2. Available tools + sections.append(self._tools_section()) + + # 3. ReAct format instructions + sections.append(_REACT_PREFIX) + + # 4. Constraints + sections.append(self._constraints_section()) + + # 5. Base system prompt from the agent definition + base_prompt = getattr(self.agent_definition, "system_prompt", "") or "" + if base_prompt: + sections.append(base_prompt) + + return "\n\n".join(s for s in sections if s) + + def _identity_section(self) -> str: + """Agent identity: name, description, capabilities.""" + name = getattr(self.agent_definition, "name", "") or "AI Agent" + description = getattr(self.agent_definition, "description", "") or "" + capabilities = getattr(self.agent_definition, "capabilities", None) or [] + + lines = [f"You are {name}."] + if description: + lines.append(f"Description: {description}") + if capabilities: + caps = ", ".join(str(c) for c in capabilities) + lines.append(f"Capabilities: {caps}") + return "\n".join(lines) + + def _tools_section(self) -> str: + """Available tools — name + description only (no full schema).""" + if not self.tool_descriptions: + return "You have no tools available. Answer from your own knowledge." + lines = ["Available tools:"] + for tool in self.tool_descriptions: + name = tool.get("name", "") + description = tool.get("description", "") + if name: + lines.append(f"- {name}: {description}") + return "\n".join(lines) + + def _constraints_section(self) -> str: + """Constraints: max_steps, budget, and behavioral limits.""" + constraints: list[str] = [] + + max_steps = self.max_steps or getattr( + self.agent_definition, "max_steps", None + ) or 20 + constraints.append(f"- Maximum {max_steps} reasoning steps per run.") + + budget = self.budget_limit_usd + if budget is None: + budget = getattr(self.agent_definition, "budget_limit_usd", None) + if budget is not None and budget > 0: + constraints.append(f"- Budget limit: ${float(budget):.2f} per run.") + + constraints.append( + "- Only call tools that are listed as available. Do not invent tools." + ) + constraints.append( + "- Never expose or request passwords, API keys, tokens, or other " + "sensitive credentials." + ) + constraints.append( + "- Respect tenant data boundaries. Do not access data outside the " + "current tenant." + ) + + return "Constraints:\n" + "\n".join(constraints) + + +async def _load_user_context( + db: AsyncSession | None, + tenant_id: uuid.UUID, + user_id: uuid.UUID, + run_as_user_id: uuid.UUID | None, +) -> dict[str, Any]: + """Load user + tenant context from the DB with graceful fallbacks.""" + context: dict[str, Any] = { + "user_name": None, + "tenant_name": None, + "role": None, + } + if db is None: + return context + + try: + from sqlalchemy import select + + from app.models.tenant import Tenant + from app.models.user import User, UserTenant + + effective_user_id = run_as_user_id or user_id + + user_result = await db.execute( + select(User).where(User.id == effective_user_id).limit(1) + ) + user = user_result.scalar_one_or_none() + if user is not None: + context["user_name"] = user.name or user.email + + tenant_result = await db.execute( + select(Tenant).where(Tenant.id == tenant_id).limit(1) + ) + tenant = tenant_result.scalar_one_or_none() + if tenant is not None: + context["tenant_name"] = tenant.name + + role_result = await db.execute( + select(UserTenant.role) + .where(UserTenant.user_id == effective_user_id) + .where(UserTenant.tenant_id == tenant_id) + .limit(1) + ) + role = role_result.scalar_one_or_none() + if role: + context["role"] = role + except Exception: + logger.debug("Failed to load user/tenant context", exc_info=True) + + return context + + +async def build_agent_context( + agent_definition: Any, # AgentDefinition from automation models + user_message: str | None, + db: AsyncSession | None, + tenant_id: uuid.UUID, + user_id: uuid.UUID, + run_as_user_id: uuid.UUID | None = None, + memory_items: list[dict] | None = None, + trace_id: str | None = None, +) -> list[dict[str, Any]]: + """Build the full message list for an agent run. + + Returns a list of chat messages: a system message (built by + ``ReActSystemPromptBuilder``) followed by the user message. Sensitive + fields are redacted from any injected context. + """ + # 1. Resolve the tools the agent has access to (filtered by tool_ids). + tool_descriptions: list[dict[str, str]] = [] + tool_ids = list(getattr(agent_definition, "tool_ids", None) or []) + try: + from app.plugins.builtins.ai_assistant.contracts import get_tool_registry + + registry = get_tool_registry() + if tool_ids: + tools = registry.get_by_names(tool_ids) + else: + tools = registry.get_all() + tool_descriptions = [ + {"name": t.name, "description": t.description} for t in tools + ] + except Exception: + logger.debug("Failed to load tool descriptions", exc_info=True) + + # 2. Build the system prompt. + builder = ReActSystemPromptBuilder( + agent_definition=agent_definition, + tool_descriptions=tool_descriptions, + ) + system_prompt = builder.build() + + # 3. Load user/tenant context. + user_ctx = await _load_user_context( + db, tenant_id, user_id, run_as_user_id + ) + + # 4. Assemble the context block (redacting sensitive fields). + context_lines: list[str] = [] + if user_ctx.get("user_name"): + context_lines.append(f"Current user: {user_ctx['user_name']}") + if user_ctx.get("tenant_name"): + context_lines.append(f"Current tenant: {user_ctx['tenant_name']}") + if user_ctx.get("role"): + context_lines.append(f"Current user role: {user_ctx['role']}") + + if memory_items: + context_lines.append("Relevant memory items:") + for item in memory_items: + content = item.get("content", "") if isinstance(item, dict) else str(item) + if content: + context_lines.append(f"- {content}") + + # 5. Build the final message list. + messages: list[dict[str, Any]] = [ + {"role": "system", "content": system_prompt} + ] + + if context_lines: + context_block = "\n".join(context_lines) + messages.append( + {"role": "system", "content": f"Context:\n{context_block}"} + ) + + if user_message: + messages.append({"role": "user", "content": user_message}) + + return messages + + +__all__ = ["ReActSystemPromptBuilder", "build_agent_context"] diff --git a/app/ai/skill_registry.py b/app/ai/skill_registry.py new file mode 100644 index 0000000..fb9990a --- /dev/null +++ b/app/ai/skill_registry.py @@ -0,0 +1,82 @@ +"""Small Skill Registry for AI agents. + +Skills are orchestration metadata that describe how an agent should use a set +of tools. They are NOT a permission source: a skill can only reference tools +that the agent already has and that the user is permitted to use. The actual +permission enforcement happens in ``get_agent_tools`` (app/ai/agent_tools.py). +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any + + +@dataclass +class SkillDefinition: + """A skill definition — orchestration metadata for a set of tools.""" + + name: str + description: str + instructions: str # How to use this skill + allowed_tool_ids: list[str] = field(default_factory=list) # Tool IDs this skill can use + context_policy: dict[str, Any] | None = None # Optional context inclusion rules + category: str = "general" + + def to_dict(self) -> dict[str, Any]: + """Serialize to a plain dict for API responses.""" + return { + "name": self.name, + "description": self.description, + "instructions": self.instructions, + "allowed_tool_ids": list(self.allowed_tool_ids or []), + "context_policy": self.context_policy, + "category": self.category, + } + + +class SkillRegistry: + """Registry for skill definitions. + + Skills are orchestration metadata, NOT a permission source. + """ + + _instance: SkillRegistry | None = None + + def __new__(cls) -> SkillRegistry: + if cls._instance is None: + cls._instance = super().__new__(cls) + cls._instance._skills: dict[str, SkillDefinition] = {} + return cls._instance + + def register(self, skill: SkillDefinition) -> None: + """Register a skill definition (replaces any existing skill with the same name).""" + self._skills[skill.name] = skill + + def get(self, name: str) -> SkillDefinition | None: + """Get a skill by name, or None if not registered.""" + return self._skills.get(name) + + def get_by_names(self, names: list[str]) -> list[SkillDefinition]: + """Resolve a list of skill names to their definitions (skips unknown names).""" + return [self._skills[name] for name in names if name in self._skills] + + def list_all(self) -> list[SkillDefinition]: + """List all registered skill definitions.""" + return list(self._skills.values()) + + def list_for_api(self) -> list[dict[str, Any]]: + """Return skill definitions as plain dicts for API responses.""" + return [skill.to_dict() for skill in self._skills.values()] + + def unregister(self, name: str) -> None: + """Remove a skill definition by name.""" + self._skills.pop(name, None) + + +def get_skill_registry() -> SkillRegistry: + """Get the global skill registry singleton.""" + return SkillRegistry() + + +__all__ = ["SkillDefinition", "SkillRegistry", "get_skill_registry"] diff --git a/app/plugins/builtins/automation/agent_routes.py b/app/plugins/builtins/automation/agent_routes.py index c6d7958..a4f5964 100644 --- a/app/plugins/builtins/automation/agent_routes.py +++ b/app/plugins/builtins/automation/agent_routes.py @@ -59,6 +59,13 @@ def _agent_to_response(a: AgentDefinition) -> AgentDefinitionResponse: max_executions_per_hour=a.max_executions_per_hour, max_duration_seconds=a.max_duration_seconds, budget_limit_usd=a.budget_limit_usd, + temperature=a.temperature, + max_tokens=a.max_tokens, + max_steps=a.max_steps, + trace_mode=a.trace_mode, + skill_ids=[str(s) for s in (a.skill_ids or [])], + trigger_config=a.trigger_config or {}, + ai_use_case_metadata=a.ai_use_case_metadata or {}, created_by=str(a.created_by) if a.created_by else None, created_at=a.created_at.isoformat() if a.created_at else None, updated_at=a.updated_at.isoformat() if a.updated_at else None, diff --git a/app/plugins/builtins/automation/migrations/0003_skill_definitions.sql b/app/plugins/builtins/automation/migrations/0003_skill_definitions.sql new file mode 100644 index 0000000..581eed7 --- /dev/null +++ b/app/plugins/builtins/automation/migrations/0003_skill_definitions.sql @@ -0,0 +1,18 @@ +-- Skill Definitions for AI Agent Skills (Phase F-SKILL) + +CREATE TABLE IF NOT EXISTS automation_skill_definitions ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + tenant_id UUID NOT NULL, + name VARCHAR(255) NOT NULL UNIQUE, + description TEXT NOT NULL, + instructions TEXT NOT NULL, + allowed_tool_ids JSONB NOT NULL DEFAULT '[]', + context_policy JSONB, + category VARCHAR(100) NOT NULL DEFAULT 'general', + is_active BOOLEAN NOT NULL DEFAULT TRUE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + deleted_at TIMESTAMPTZ +); +CREATE INDEX IF NOT EXISTS ix_skill_defs_tenant_active ON automation_skill_definitions (tenant_id, is_active); +CREATE INDEX IF NOT EXISTS ix_skill_defs_tenant_category ON automation_skill_definitions (tenant_id, category); diff --git a/app/plugins/builtins/automation/models.py b/app/plugins/builtins/automation/models.py index 098554e..abdd305 100644 --- a/app/plugins/builtins/automation/models.py +++ b/app/plugins/builtins/automation/models.py @@ -62,6 +62,19 @@ class AgentDefinition(Base, TenantMixin, OwnedMixin): budget_limit_usd: Mapped[float] = mapped_column( Float, nullable=False, default=1.0 ) + temperature: Mapped[float] = mapped_column(Float, nullable=False, default=0.3) + max_tokens: Mapped[int] = mapped_column(Integer, nullable=False, default=1000) + max_steps: Mapped[int] = mapped_column(Integer, nullable=False, default=20) + trace_mode: Mapped[str] = mapped_column( + String(20), nullable=False, default="standard" + ) + skill_ids: Mapped[list[Any]] = mapped_column(JSONB, nullable=False, default=list) + trigger_config: Mapped[dict[str, Any]] = mapped_column( + JSONB, nullable=False, default=dict + ) + ai_use_case_metadata: Mapped[dict[str, Any]] = mapped_column( + JSONB, nullable=False, default=dict + ) created_by: Mapped[uuid.UUID | None] = mapped_column( PGUUID(as_uuid=True), ForeignKey("users.id", ondelete="SET NULL"), nullable=True ) @@ -318,3 +331,34 @@ class AgentSubtask(Base, TenantMixin): completed_at: Mapped[datetime | None] = mapped_column( DateTime(timezone=True), nullable=True ) + + +class SkillDefinitionDB(Base, TenantMixin): + """A skill definition persisted per tenant. + + Skills are orchestration metadata, NOT a permission source. They reference + tool IDs that the agent and the user must already be permitted to use. + """ + + __tablename__ = "automation_skill_definitions" + __table_args__ = ( + Index("ix_skill_defs_tenant_active", "tenant_id", "is_active"), + Index("ix_skill_defs_tenant_category", "tenant_id", "category"), + ) + + id: Mapped[uuid.UUID] = mapped_column( + PGUUID(as_uuid=True), primary_key=True, default=uuid.uuid4 + ) + name: Mapped[str] = mapped_column(String(255), nullable=False, unique=True) + description: Mapped[str] = mapped_column(Text, nullable=False) + instructions: Mapped[str] = mapped_column(Text, nullable=False) + allowed_tool_ids: Mapped[list[Any]] = mapped_column(JSONB, nullable=False, default=list) + context_policy: Mapped[dict[str, Any] | None] = mapped_column(JSONB, nullable=True) + category: Mapped[str] = mapped_column(String(100), nullable=False, default="general") + is_active: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, server_default=func.now() + ) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, server_default=func.now(), onupdate=func.now() + ) diff --git a/app/plugins/builtins/automation/plugin.py b/app/plugins/builtins/automation/plugin.py index 4c82fa7..e390423 100644 --- a/app/plugins/builtins/automation/plugin.py +++ b/app/plugins/builtins/automation/plugin.py @@ -50,6 +50,11 @@ class AutomationPlugin(BasePlugin): module="app.plugins.builtins.automation.agent_routes", router_attr="router", ), + PluginRouteDef( + path="/api/v1/skills", + module="app.plugins.builtins.automation.skill_routes", + router_attr="router", + ), ], events=[ "contact.created", @@ -57,7 +62,7 @@ class AutomationPlugin(BasePlugin): "mail.received", "workflow.timeout", ], - migrations=["0001_initial.sql", "0002_agent_subtasks.sql"], + migrations=["0001_initial.sql", "0002_agent_subtasks.sql", "0003_skill_definitions.sql"], permissions=[ "automation:read", "automation:write", diff --git a/app/plugins/builtins/automation/schemas.py b/app/plugins/builtins/automation/schemas.py index 821c96d..4576828 100644 --- a/app/plugins/builtins/automation/schemas.py +++ b/app/plugins/builtins/automation/schemas.py @@ -23,6 +23,13 @@ class AgentDefinitionCreate(BaseModel): max_executions_per_hour: int = Field(default=10, ge=1, le=1000) max_duration_seconds: int = Field(default=300, ge=1, le=86400) budget_limit_usd: float = Field(default=1.0, ge=0.0, le=10000.0) + temperature: float = Field(default=0.3, ge=0.0, le=2.0) + max_tokens: int = Field(default=1000, ge=1, le=100000) + max_steps: int = Field(default=20, ge=1, le=100) + trace_mode: str = Field(default="standard", pattern="^(standard|extended)$") + skill_ids: list[str] = Field(default_factory=list) + trigger_config: dict[str, Any] = Field(default_factory=dict) + ai_use_case_metadata: dict[str, Any] = Field(default_factory=dict) class AgentDefinitionUpdate(BaseModel): @@ -326,3 +333,52 @@ class SubtaskListResponse(BaseModel): items: list[SubtaskRead] total: int + + +# ─── Skill Definition Schemas (Phase F-SKILL) ─── + + +class SkillDefinitionCreate(BaseModel): + """Create a new skill definition.""" + + name: str = Field(..., min_length=1, max_length=255) + description: str = Field(..., min_length=1) + instructions: str = Field(..., min_length=1) + allowed_tool_ids: list[str] = Field(default_factory=list) + context_policy: dict[str, Any] | None = None + category: str = Field(default="general", max_length=100) + is_active: bool = Field(default=True) + + +class SkillDefinitionUpdate(BaseModel): + """Update an existing skill definition (partial).""" + + name: str | None = Field(None, min_length=1, max_length=255) + description: str | None = Field(None, min_length=1) + instructions: str | None = Field(None, min_length=1) + allowed_tool_ids: list[str] | None = None + context_policy: dict[str, Any] | None = None + category: str | None = Field(None, max_length=100) + is_active: bool | None = None + + +class SkillDefinitionResponse(BaseModel): + """Skill definition response.""" + + id: str + name: str + description: str + instructions: str + allowed_tool_ids: list[str] = [] + context_policy: dict[str, Any] | None = None + category: str = "general" + is_active: bool = True + created_at: str | None = None + updated_at: str | None = None + + +class SkillDefinitionListResponse(BaseModel): + """Paginated skill definition list.""" + + items: list[SkillDefinitionResponse] + total: int diff --git a/app/plugins/builtins/automation/services.py b/app/plugins/builtins/automation/services.py index 2de7051..1588dc6 100644 --- a/app/plugins/builtins/automation/services.py +++ b/app/plugins/builtins/automation/services.py @@ -22,6 +22,7 @@ from app.plugins.builtins.automation.models import ( AutomationDefinition, AutomationRun, AutomationVersion, + SkillDefinitionDB, ) logger = logging.getLogger(__name__) @@ -114,6 +115,13 @@ class AgentService: max_executions_per_hour=data.get("max_executions_per_hour", 10), max_duration_seconds=data.get("max_duration_seconds", 300), budget_limit_usd=data.get("budget_limit_usd", 1.0), + temperature=data.get("temperature", 0.3), + max_tokens=data.get("max_tokens", 1000), + max_steps=data.get("max_steps", 20), + trace_mode=data.get("trace_mode", "standard"), + skill_ids=data.get("skill_ids", []), + trigger_config=data.get("trigger_config", {}), + ai_use_case_metadata=data.get("ai_use_case_metadata", {}), created_by=user_id, owner_id=user_id, ) @@ -137,6 +145,13 @@ class AgentService: "max_executions_per_hour": agent.max_executions_per_hour, "max_duration_seconds": agent.max_duration_seconds, "budget_limit_usd": agent.budget_limit_usd, + "temperature": agent.temperature, + "max_tokens": agent.max_tokens, + "max_steps": agent.max_steps, + "trace_mode": agent.trace_mode, + "skill_ids": agent.skill_ids, + "trigger_config": agent.trigger_config, + "ai_use_case_metadata": agent.ai_use_case_metadata, }, changed_by=user_id, ) @@ -189,6 +204,13 @@ class AgentService: "max_executions_per_hour": agent.max_executions_per_hour, "max_duration_seconds": agent.max_duration_seconds, "budget_limit_usd": agent.budget_limit_usd, + "temperature": agent.temperature, + "max_tokens": agent.max_tokens, + "max_steps": agent.max_steps, + "trace_mode": agent.trace_mode, + "skill_ids": agent.skill_ids, + "trigger_config": agent.trigger_config, + "ai_use_case_metadata": agent.ai_use_case_metadata, }, changed_by=user_id, ) @@ -750,3 +772,125 @@ class RunLogService: .offset(offset) ) return list(result.scalars().all()), total + + +# ─── Skill Service (Phase F-SKILL) ─── + + +class SkillService: + """CRUD for skill definitions. + + Skills are orchestration metadata, NOT a permission source. They reference + tool IDs that the agent and the user must already be permitted to use. + """ + + @staticmethod + async def list( + db: AsyncSession, + tenant_id: uuid.UUID, + is_active: bool | None = None, + category: str | None = None, + limit: int = 50, + offset: int = 0, + ) -> tuple[list[SkillDefinitionDB], int]: + """List skill definitions with optional filters.""" + query = select(SkillDefinitionDB).where( + SkillDefinitionDB.tenant_id == tenant_id + ) + count_query = ( + select(func.count()) + .select_from(SkillDefinitionDB) + .where(SkillDefinitionDB.tenant_id == tenant_id) + ) + + if is_active is not None: + query = query.where(SkillDefinitionDB.is_active == is_active) + count_query = count_query.where(SkillDefinitionDB.is_active == is_active) + if category is not None: + query = query.where(SkillDefinitionDB.category == category) + count_query = count_query.where(SkillDefinitionDB.category == category) + + count_result = await db.execute(count_query) + total = count_result.scalar() or 0 + + result = await db.execute( + query.order_by(SkillDefinitionDB.created_at.desc()) + .limit(limit) + .offset(offset) + ) + return list(result.scalars().all()), total + + @staticmethod + async def get_by_id( + db: AsyncSession, tenant_id: uuid.UUID, skill_id: uuid.UUID + ) -> SkillDefinitionDB | None: + """Get a single skill definition by ID.""" + result = await db.execute( + select(SkillDefinitionDB) + .where(SkillDefinitionDB.id == skill_id) + .where(SkillDefinitionDB.tenant_id == tenant_id) + .limit(1) + ) + return result.scalar_one_or_none() + + @staticmethod + async def get_by_name( + db: AsyncSession, name: str, tenant_id: uuid.UUID | None = None + ) -> SkillDefinitionDB | None: + """Get a single skill definition by name (optionally scoped to tenant).""" + query = select(SkillDefinitionDB).where(SkillDefinitionDB.name == name) + if tenant_id is not None: + query = query.where(SkillDefinitionDB.tenant_id == tenant_id) + result = await db.execute(query.limit(1)) + return result.scalar_one_or_none() + + @staticmethod + async def create( + db: AsyncSession, + tenant_id: uuid.UUID, + data: dict[str, Any], + ) -> SkillDefinitionDB: + """Create a new skill definition.""" + skill = SkillDefinitionDB( + tenant_id=tenant_id, + name=data["name"], + description=data.get("description", ""), + instructions=data.get("instructions", ""), + allowed_tool_ids=data.get("allowed_tool_ids", []), + context_policy=data.get("context_policy"), + category=data.get("category", "general"), + is_active=data.get("is_active", True), + ) + db.add(skill) + await db.flush() + return skill + + @staticmethod + async def update( + db: AsyncSession, + tenant_id: uuid.UUID, + skill_id: uuid.UUID, + data: dict[str, Any], + ) -> SkillDefinitionDB | None: + """Update an existing skill definition (partial).""" + skill = await SkillService.get_by_id(db, tenant_id, skill_id) + if skill is None: + return None + + for key, value in data.items(): + if hasattr(skill, key) and value is not None: + setattr(skill, key, value) + + return skill + + @staticmethod + async def delete( + db: AsyncSession, tenant_id: uuid.UUID, skill_id: uuid.UUID + ) -> bool: + """Delete a skill definition.""" + skill = await SkillService.get_by_id(db, tenant_id, skill_id) + if skill is None: + return False + await db.delete(skill) + await db.flush() + return True diff --git a/app/plugins/builtins/automation/skill_routes.py b/app/plugins/builtins/automation/skill_routes.py new file mode 100644 index 0000000..159373d --- /dev/null +++ b/app/plugins/builtins/automation/skill_routes.py @@ -0,0 +1,164 @@ +"""API routes for skill definitions — /api/v1/skills. + +Endpoints: skill definitions CRUD. Skills are orchestration metadata, NOT a +permission source: they reference tool IDs that the agent and the user must +already be permitted to use. +""" + +from __future__ import annotations + +import logging +import uuid +from typing import Any + +from fastapi import APIRouter, Depends, HTTPException, Query +from sqlalchemy.ext.asyncio import AsyncSession + +from app.core.db import get_db +from app.deps import get_current_user, require_permission +from app.plugins.builtins.automation.models import SkillDefinitionDB +from app.plugins.builtins.automation.schemas import ( + SkillDefinitionCreate, + SkillDefinitionListResponse, + SkillDefinitionResponse, + SkillDefinitionUpdate, +) +from app.plugins.builtins.automation.services import SkillService + +logger = logging.getLogger(__name__) + +router = APIRouter(prefix="/api/v1/skills", tags=["skills"]) + + +# ─── Helper Functions ─── + + +def _skill_to_response(s: SkillDefinitionDB) -> SkillDefinitionResponse: + """Convert SkillDefinitionDB model to response schema.""" + return SkillDefinitionResponse( + id=str(s.id), + name=s.name, + description=s.description or "", + instructions=s.instructions or "", + allowed_tool_ids=[str(t) for t in (s.allowed_tool_ids or [])], + context_policy=s.context_policy, + category=s.category or "general", + is_active=s.is_active, + created_at=s.created_at.isoformat() if s.created_at else None, + updated_at=s.updated_at.isoformat() if s.updated_at else None, + ) + + +# ─── CRUD Endpoints ─── + + +@router.get( + "", + dependencies=[Depends(require_permission("automation:read"))], + response_model=SkillDefinitionListResponse, +) +async def list_skills( + is_active: bool | None = Query(None), + category: str | None = Query(None), + limit: int = Query(50, ge=1, le=200), + offset: int = Query(0, ge=0), + current_user: dict[str, Any] = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + """List skill definitions with optional filters.""" + tenant_id = uuid.UUID(current_user["tenant_id"]) + items, total = await SkillService.list( + db, tenant_id, is_active=is_active, category=category, limit=limit, offset=offset + ) + return SkillDefinitionListResponse( + items=[_skill_to_response(s) for s in items], + total=total, + ) + + +@router.post( + "/", + dependencies=[Depends(require_permission("automation:write"))], + response_model=SkillDefinitionResponse, + status_code=201, +) +async def create_skill( + data: SkillDefinitionCreate, + current_user: dict[str, Any] = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + """Create a new skill definition.""" + tenant_id = uuid.UUID(current_user["tenant_id"]) + skill = await SkillService.create(db, tenant_id, data.model_dump()) + return _skill_to_response(skill) + + +@router.get( + "/{skill_id}", + dependencies=[Depends(require_permission("automation:read"))], + response_model=SkillDefinitionResponse, +) +async def get_skill( + skill_id: str, + current_user: dict[str, Any] = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + """Get a single skill definition by ID.""" + tenant_id = uuid.UUID(current_user["tenant_id"]) + try: + sid = uuid.UUID(skill_id) + except (ValueError, TypeError): + raise HTTPException(status_code=400, detail="Invalid skill ID") from None + + skill = await SkillService.get_by_id(db, tenant_id, sid) + if skill is None: + raise HTTPException(status_code=404, detail="Skill not found") + return _skill_to_response(skill) + + +@router.patch( + "/{skill_id}", + dependencies=[Depends(require_permission("automation:write"))], + response_model=SkillDefinitionResponse, +) +async def update_skill( + skill_id: str, + data: SkillDefinitionUpdate, + current_user: dict[str, Any] = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + """Update an existing skill definition.""" + tenant_id = uuid.UUID(current_user["tenant_id"]) + try: + sid = uuid.UUID(skill_id) + except (ValueError, TypeError): + raise HTTPException(status_code=400, detail="Invalid skill ID") from None + + skill = await SkillService.update( + db, tenant_id, sid, data.model_dump(exclude_none=True) + ) + if skill is None: + raise HTTPException(status_code=404, detail="Skill not found") + return _skill_to_response(skill) + + +@router.delete( + "/{skill_id}", + dependencies=[Depends(require_permission("automation:delete"))], +) +async def delete_skill( + skill_id: str, + current_user: dict[str, Any] = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + """Delete a skill definition.""" + tenant_id = uuid.UUID(current_user["tenant_id"]) + try: + sid = uuid.UUID(skill_id) + except (ValueError, TypeError): + raise HTTPException(status_code=400, detail="Invalid skill ID") from None + + success = await SkillService.delete(db, tenant_id, sid) + if not success: + raise HTTPException(status_code=404, detail="Skill not found") + return {"status": "ok"} diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py new file mode 100644 index 0000000..aeb3057 --- /dev/null +++ b/tests/test_agent_tools.py @@ -0,0 +1,219 @@ +"""Tests for Tool-/Skill-Binding (app/ai/agent_tools.py). + +All tests use mocked tool/skill registries — no real DB or LLM needed. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any + +import pytest + +from app.ai.agent_tools import get_agent_tools +from app.ai.skill_registry import SkillDefinition, SkillRegistry + + +@dataclass +class MockTool: + """Minimal stand-in for AITool.""" + + name: str + description: str = "" + parameters: dict[str, Any] = field(default_factory=dict) + required_permission: str | None = None + + def to_openai_schema(self) -> dict[str, Any]: + return { + "type": "function", + "function": { + "name": self.name, + "description": self.description, + "parameters": self.parameters, + }, + } + + +class MockToolRegistry: + """In-memory tool registry with get_by_names.""" + + def __init__(self, tools: list[MockTool]) -> None: + self._tools = {t.name: t for t in tools} + + def get_by_names(self, names: list[str]) -> list[MockTool]: + return [self._tools[n] for n in names if n in self._tools] + + +@dataclass +class MockAgentDefinition: + """Minimal stand-in for AgentDefinition.""" + + tool_ids: list[str] = field(default_factory=list) + skill_ids: list[str] = field(default_factory=list) + + +@pytest.fixture(autouse=True) +def _clean_skill_registry(): + """Reset the singleton skill registry between tests.""" + registry = SkillRegistry() + for skill in list(registry.list_all()): + registry.unregister(skill.name) + yield + for skill in list(registry.list_all()): + registry.unregister(skill.name) + + +def _make_skill( + name: str, + allowed_tool_ids: list[str], +) -> SkillDefinition: + return SkillDefinition( + name=name, + description="test skill", + instructions="use the tools", + allowed_tool_ids=allowed_tool_ids, + ) + + +def _permissions( + permissions: list[str] | None = None, + denied: list[str] | None = None, + is_system_admin: bool = False, +) -> dict[str, Any]: + return { + "permissions": permissions or [], + "denied_permissions": denied or [], + "is_system_admin": is_system_admin, + } + + +class TestGetAgentTools: + def test_direct_tools_available(self): + """Tools directly on the agent are available (no skills).""" + registry = MockToolRegistry([ + MockTool("mail_read", required_permission="mail:read"), + MockTool("contact_list"), + ]) + skill_registry = SkillRegistry() + agent = MockAgentDefinition(tool_ids=["mail_read", "contact_list"]) + + schemas, skills = get_agent_tools( + agent, registry, skill_registry, _permissions(permissions=["mail:read"]) + ) + + assert len(schemas) == 2 + names = {s["function"]["name"] for s in schemas} + assert names == {"mail_read", "contact_list"} + assert skills == [] + + def test_skill_tools_intersect_with_agent_tools(self): + """Skill tools are only available if also in agent.tool_ids.""" + registry = MockToolRegistry([ + MockTool("mail_read", required_permission="mail:read"), + MockTool("mail_list", required_permission="mail:read"), + MockTool("secret_tool"), + ]) + skill_registry = SkillRegistry() + skill_registry.register(_make_skill("mail_skill", ["mail_read", "mail_list", "secret_tool"])) + # Agent has mail_read + mail_list, but NOT secret_tool + agent = MockAgentDefinition( + tool_ids=["mail_read", "mail_list"], + skill_ids=["mail_skill"], + ) + + schemas, skills = get_agent_tools( + agent, registry, skill_registry, _permissions(permissions=["mail:read"]) + ) + + names = {s["function"]["name"] for s in schemas} + assert names == {"mail_read", "mail_list"} + assert "secret_tool" not in names + assert len(skills) == 1 + assert skills[0].name == "mail_skill" + + def test_skill_never_grants_permission(self): + """A skill cannot grant access to a tool the user lacks permission for.""" + registry = MockToolRegistry([ + MockTool("mail_read", required_permission="mail:read"), + ]) + skill_registry = SkillRegistry() + skill_registry.register(_make_skill("mail_skill", ["mail_read"])) + agent = MockAgentDefinition(tool_ids=["mail_read"], skill_ids=["mail_skill"]) + + # User does NOT have mail:read + schemas, skills = get_agent_tools( + agent, registry, skill_registry, _permissions(permissions=[]) + ) + + assert schemas == [] + assert len(skills) == 1 # skill is still resolved, but grants no tools + + def test_denied_permission_blocks_tool(self): + """Explicitly denied permissions block a tool even if granted.""" + registry = MockToolRegistry([ + MockTool("mail_read", required_permission="mail:read"), + ]) + skill_registry = SkillRegistry() + agent = MockAgentDefinition(tool_ids=["mail_read"]) + + schemas, _ = get_agent_tools( + agent, + registry, + skill_registry, + _permissions(permissions=["mail:read"], denied=["mail:read"]), + ) + + assert schemas == [] + + def test_system_admin_gets_all_tools(self): + """System admins bypass permission checks.""" + registry = MockToolRegistry([ + MockTool("mail_read", required_permission="mail:read"), + ]) + skill_registry = SkillRegistry() + agent = MockAgentDefinition(tool_ids=["mail_read"]) + + schemas, _ = get_agent_tools( + agent, registry, skill_registry, _permissions(is_system_admin=True) + ) + + assert len(schemas) == 1 + + def test_wildcard_permission_matches(self): + """Wildcard permissions (mail:*) satisfy a required permission.""" + registry = MockToolRegistry([ + MockTool("mail_read", required_permission="mail:read"), + ]) + skill_registry = SkillRegistry() + agent = MockAgentDefinition(tool_ids=["mail_read"]) + + schemas, _ = get_agent_tools( + agent, registry, skill_registry, _permissions(permissions=["mail:*"]) + ) + + assert len(schemas) == 1 + + def test_tool_without_permission_always_available(self): + """Tools without a required_permission are always available.""" + registry = MockToolRegistry([MockTool("contact_list")]) + skill_registry = SkillRegistry() + agent = MockAgentDefinition(tool_ids=["contact_list"]) + + schemas, _ = get_agent_tools( + agent, registry, skill_registry, _permissions(permissions=[]) + ) + + assert len(schemas) == 1 + + def test_unknown_tools_are_skipped(self): + """Tool IDs not in the registry are silently skipped.""" + registry = MockToolRegistry([MockTool("known_tool")]) + skill_registry = SkillRegistry() + agent = MockAgentDefinition(tool_ids=["known_tool", "missing_tool"]) + + schemas, _ = get_agent_tools( + agent, registry, skill_registry, _permissions(permissions=[]) + ) + + assert len(schemas) == 1 + assert schemas[0]["function"]["name"] == "known_tool" diff --git a/tests/test_skill_registry.py b/tests/test_skill_registry.py new file mode 100644 index 0000000..b64bd64 --- /dev/null +++ b/tests/test_skill_registry.py @@ -0,0 +1,97 @@ +"""Tests for the Small Skill Registry (app/ai/skill_registry.py).""" + +from __future__ import annotations + +import pytest + +from app.ai.skill_registry import SkillDefinition, get_skill_registry + + +@pytest.fixture(autouse=True) +def _clean_registry(): + """Reset the singleton registry between tests.""" + registry = get_skill_registry() + for skill in list(registry.list_all()): + registry.unregister(skill.name) + yield + for skill in list(registry.list_all()): + registry.unregister(skill.name) + + +def _make_skill( + name: str = "mail_reader", + allowed_tool_ids: list[str] | None = None, + category: str = "general", +) -> SkillDefinition: + return SkillDefinition( + name=name, + description="Read and summarize emails", + instructions="Use the mail tools to read and summarize emails.", + allowed_tool_ids=allowed_tool_ids or ["mail_read", "mail_list"], + context_policy={"include_recent": True}, + category=category, + ) + + +class TestSkillDefinition: + def test_defaults(self): + skill = SkillDefinition( + name="s", description="d", instructions="i", allowed_tool_ids=["t"] + ) + assert skill.context_policy is None + assert skill.category == "general" + + def test_to_dict(self): + skill = _make_skill() + data = skill.to_dict() + assert data["name"] == "mail_reader" + assert data["allowed_tool_ids"] == ["mail_read", "mail_list"] + assert data["context_policy"] == {"include_recent": True} + assert data["category"] == "general" + + +class TestSkillRegistry: + def test_register_and_get(self): + registry = get_skill_registry() + skill = _make_skill() + registry.register(skill) + assert registry.get("mail_reader") is skill + + def test_get_missing_returns_none(self): + registry = get_skill_registry() + assert registry.get("nope") is None + + def test_get_by_names_skips_unknown(self): + registry = get_skill_registry() + a = _make_skill("a") + b = _make_skill("b") + registry.register(a) + registry.register(b) + resolved = registry.get_by_names(["a", "b", "missing"]) + assert len(resolved) == 2 + assert {s.name for s in resolved} == {"a", "b"} + + def test_list_all_and_list_for_api(self): + registry = get_skill_registry() + registry.register(_make_skill("a")) + registry.register(_make_skill("b")) + assert len(registry.list_all()) == 2 + api = registry.list_for_api() + assert len(api) == 2 + assert all("name" in item and "instructions" in item for item in api) + + def test_unregister(self): + registry = get_skill_registry() + registry.register(_make_skill()) + registry.unregister("mail_reader") + assert registry.get("mail_reader") is None + + def test_register_replaces_existing(self): + registry = get_skill_registry() + registry.register(_make_skill("a", allowed_tool_ids=["t1"])) + registry.register(_make_skill("a", allowed_tool_ids=["t2"])) + assert registry.get("a").allowed_tool_ids == ["t2"] + assert len(registry.list_all()) == 1 + + def test_singleton(self): + assert get_skill_registry() is get_skill_registry()