diff --git a/alembic/versions/a7_002_merge_heads.py b/alembic/versions/a7_002_merge_heads.py new file mode 100644 index 000000000..e01b71cdd --- /dev/null +++ b/alembic/versions/a7_002_merge_heads.py @@ -0,0 +1,28 @@ +"""Merge session persistence and resource/automation heads. + +Revision ID: a7_002_merge_heads +Revises: 71cd40eb661f, a7_001_session_persistence +Create Date: 2026-02-18 04:30:00 + +""" + +from collections.abc import Sequence + +# revision identifiers, used by Alembic. +revision: str = "a7_002_merge_heads" +down_revision: str | Sequence[str] | None = ( + "71cd40eb661f", + "a7_001_session_persistence", +) +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + """Upgrade schema.""" + pass + + +def downgrade() -> None: + """Downgrade schema.""" + pass diff --git a/src/cleveragents/application/services/tool_registry_service.py b/src/cleveragents/application/services/tool_registry_service.py index 4d4d645f5..a87355980 100644 --- a/src/cleveragents/application/services/tool_registry_service.py +++ b/src/cleveragents/application/services/tool_registry_service.py @@ -9,9 +9,8 @@ from __future__ import annotations from typing import Any -from cleveragents.core.exceptions import ( - NotFoundError, -) +from cleveragents.core.exceptions import NotFoundError, ValidationError +from cleveragents.domain.models.core.tool import Tool from cleveragents.infrastructure.database.repositories import ( ToolRegistryRepository, ValidationAttachmentRepository, @@ -49,13 +48,22 @@ class ToolRegistryService: DuplicateToolError: If the name already exists. DatabaseError: On persistence failure. """ + create_fn = getattr(self._tool_repo, "create", None) + if callable(create_fn): + return create_fn(tool) + add_fn = getattr(self._tool_repo, "add", None) + if callable(add_fn): + return add_fn(tool) return self._tool_repo.create(tool) - def update_tool(self, tool: Any) -> Any: + def update_tool( + self, tool: Any, tool_config: dict[str, Any] | Tool | None = None + ) -> Any: """Update an existing tool definition. Args: tool: A tool dict or domain object with updated fields. + tool_config: Optional tool configuration for name-based update. Returns: The updated tool. @@ -63,7 +71,15 @@ class ToolRegistryService: Raises: DatabaseError: If the tool is not found or persistence fails. """ - return self._tool_repo.update(tool) + if tool_config is None: + return self._tool_repo.update(tool) + + if isinstance(tool_config, Tool): + updated = tool_config + else: + updated = {"name": tool, **tool_config} if isinstance(tool, str) else tool + + return self._tool_repo.update(updated) def remove_tool(self, name: str) -> bool: """Remove a tool from the registry. @@ -78,6 +94,14 @@ class ToolRegistryService: ToolInUseError: If validation attachments still reference this tool. """ + delete_fn = getattr(self._tool_repo, "delete", None) + if callable(delete_fn): + return bool(delete_fn(name)) + + remove_fn = getattr(self._tool_repo, "remove", None) + if callable(remove_fn): + return bool(remove_fn(name)) + return self._tool_repo.delete(name) def list_tools( @@ -141,6 +165,9 @@ class ToolRegistryService: NotFoundError: If the validation tool does not exist. DatabaseError: On persistence failure. """ + if mode not in {"required", "informational"}: + raise ValidationError(f"Invalid mode '{mode}'") + # Verify validation exists existing = self._tool_repo.get_by_name(validation_name) if existing is None: diff --git a/src/cleveragents/infrastructure/database/__init__.py b/src/cleveragents/infrastructure/database/__init__.py index 2d297bb64..a24b90c26 100644 --- a/src/cleveragents/infrastructure/database/__init__.py +++ b/src/cleveragents/infrastructure/database/__init__.py @@ -19,6 +19,7 @@ from .models import ( ProjectModel, ProjectResourceLinkModel, ResourceEdgeModel, + ResourceLinkModel, ResourceModel, ResourceTypeModel, SessionMessageModel, @@ -35,15 +36,18 @@ from .repositories import ( ActionRepository, ChangeRepository, ContextRepository, + CycleDetectedError, DuplicateActionError, DuplicateLinkError, DuplicatePlanError, DuplicateResourceError, + DuplicateResourceLinkError, DuplicateResourceTypeError, DuplicateToolError, DuplicateValidationAttachmentError, InvalidToolTypeError, LifecyclePlanRepository, + LinkNotFoundError, NamespacedProjectRepository, PlanNotFoundError, PlanRepository, @@ -62,6 +66,7 @@ from .repositories import ( ToolNotFoundError, ToolRegistryRepository, ToolRepository, + TypeIncompatibleError, ValidationAttachmentRepository, ) from .unit_of_work import UnitOfWork, UnitOfWorkContext @@ -76,10 +81,12 @@ __all__ = [ "ChangeRepository", "ContextModel", "ContextRepository", + "CycleDetectedError", "DuplicateActionError", "DuplicateLinkError", "DuplicatePlanError", "DuplicateResourceError", + "DuplicateResourceLinkError", "DuplicateResourceTypeError", "DuplicateToolError", "DuplicateValidationAttachmentError", @@ -87,6 +94,7 @@ __all__ = [ "LifecycleActionModel", "LifecyclePlanModel", "LifecyclePlanRepository", + "LinkNotFoundError", "NamespacedProjectModel", "NamespacedProjectRepository", "PlanArgumentModel", @@ -102,6 +110,7 @@ __all__ = [ "ProjectResourceLinkRepository", "ResourceEdgeModel", "ResourceHasEdgesError", + "ResourceLinkModel", "ResourceModel", "ResourceNotFoundRepoError", "ResourceRepository", @@ -120,6 +129,7 @@ __all__ = [ "ToolRegistryRepository", "ToolRepository", "ToolResourceBindingModel", + "TypeIncompatibleError", "UnitOfWork", "UnitOfWorkContext", "ValidationAttachmentModel", diff --git a/src/cleveragents/infrastructure/database/models.py b/src/cleveragents/infrastructure/database/models.py index fcc738b4d..c5e327c9f 100644 --- a/src/cleveragents/infrastructure/database/models.py +++ b/src/cleveragents/infrastructure/database/models.py @@ -47,7 +47,7 @@ from sqlalchemy import ( UniqueConstraint, create_engine, ) -from sqlalchemy.orm import declarative_base, relationship, sessionmaker +from sqlalchemy.orm import declarative_base, relationship, sessionmaker, synonym from cleveragents.domain.models.core import ( ContextType, @@ -1463,6 +1463,51 @@ class ResourceEdgeModel(Base): # type: ignore[misc] ) +# --------------------------------------------------------------------------- +# Resource Link Models (Stage B1 - migration b1_001_resource_links) +# --------------------------------------------------------------------------- + + +class ResourceLinkModel(Base): # type: ignore[misc] + """Database model for validated resource DAG links. + + Stores parent-child links between resources after validation + (cycle detection, type compatibility). Unlike ``resource_edges`` + which stores raw DAG edges with link-type metadata, this table + records validated DAG relationships managed by ``link_child`` / + ``unlink_child``. + + Table: ``resource_links`` + """ + + __allow_unmapped__ = True + __tablename__ = "resource_links" + + # Composite PK: (parent_id, child_id) + parent_id = Column( + String(26), + ForeignKey("resources.resource_id", ondelete="CASCADE"), + primary_key=True, + ) + child_id = Column( + String(26), + ForeignKey("resources.resource_id", ondelete="CASCADE"), + primary_key=True, + ) + + # Timestamp (ISO-8601 string) + created_at = Column(String(30), nullable=False) + + __table_args__ = ( + CheckConstraint( + "parent_id != child_id", + name="ck_resource_links_no_self_loop", + ), + Index("ix_resource_links_child", "child_id"), + Index("ix_resource_links_parent", "parent_id"), + ) + + # --------------------------------------------------------------------------- # Tool Registry Models (Stage C1 - migration c1_001) # --------------------------------------------------------------------------- @@ -1484,6 +1529,7 @@ class ToolModel(Base): # type: ignore[misc] # PK: namespaced name (e.g. "local/lint-check") name = Column(String(255), primary_key=True) + tool_id = synonym("name") namespace = Column(String(100), nullable=False) short_name = Column(String(150), nullable=False) @@ -1523,6 +1569,16 @@ class ToolModel(Base): # type: ignore[misc] created_at = Column(String(30), nullable=False) updated_at = Column(String(30), nullable=False) + @property + def input_schema(self) -> dict[str, Any] | None: + raw = cast("str | None", self.input_schema_json) + return json.loads(raw) if raw else None + + @property + def output_schema(self) -> dict[str, Any] | None: + raw = cast("str | None", self.output_schema_json) + return json.loads(raw) if raw else None + # Relationships resource_bindings_rel = relationship( "ToolResourceBindingModel", @@ -1677,6 +1733,7 @@ class ToolResourceBindingModel(Base): # type: ignore[misc] ForeignKey("tools.name", ondelete="CASCADE"), nullable=False, ) + tool_id = synonym("tool_name") slot_name = Column(String(100), nullable=False) resource_type = Column(String(255), nullable=False) access_mode = Column(String(20), nullable=False) diff --git a/src/cleveragents/infrastructure/database/repositories.py b/src/cleveragents/infrastructure/database/repositories.py index c7f346db0..29f8d666b 100644 --- a/src/cleveragents/infrastructure/database/repositories.py +++ b/src/cleveragents/infrastructure/database/repositories.py @@ -90,6 +90,7 @@ from cleveragents.infrastructure.database.models import ( ProjectModel, ProjectResourceLinkModel, ResourceEdgeModel, + ResourceLinkModel, ResourceModel, ResourceTypeModel, SessionMessageModel, @@ -1562,6 +1563,48 @@ class DuplicateResourceError(DatabaseError): self.resource_name = name +class CycleDetectedError(BusinessRuleViolation): + """Raised when a resource link would create a cycle.""" + + def __init__(self, parent_id: str, child_id: str, path: list[str]): + cycle_str = " -> ".join(path) + super().__init__( + f"Linking {parent_id} -> {child_id} would create a cycle: {cycle_str}" + ) + self.parent_id = parent_id + self.child_id = child_id + self.path = path + + +class TypeIncompatibleError(BusinessRuleViolation): + """Raised when a child resource type is not allowed by the parent type.""" + + def __init__(self, parent_type: str, child_type: str): + super().__init__( + f"Type '{child_type}' is not an allowed child of '{parent_type}'" + ) + self.parent_type = parent_type + self.child_type = child_type + + +class LinkNotFoundError(DatabaseError): + """Raised when a resource link cannot be found.""" + + def __init__(self, parent_id: str, child_id: str): + super().__init__(f"Link from '{parent_id}' to '{child_id}' not found") + self.parent_id = parent_id + self.child_id = child_id + + +class DuplicateResourceLinkError(DatabaseError): + """Raised when creating a duplicate resource link.""" + + def __init__(self, parent_id: str, child_id: str): + super().__init__(f"Link from '{parent_id}' to '{child_id}' already exists") + self.parent_id = parent_id + self.child_id = child_id + + class ResourceTypeRepository: """Repository for resource type persistence. @@ -2175,6 +2218,430 @@ class ResourceRepository: f"Failed to delete resource '{resource_id}': {exc}" ) from exc + @database_retry + def link_child(self, parent_id: str, child_id: str) -> None: + """Link a child resource to a parent in the DAG. + + Validates both resources exist, checks type compatibility + (child's type must be in parent type's ``child_types``), + and detects cycles before persisting. + + Args: + parent_id: ULID of the parent resource. + child_id: ULID of the child resource. + + Raises: + ResourceNotFoundRepoError: If either resource is missing. + TypeIncompatibleError: If child type is not allowed. + CycleDetectedError: If the link would create a cycle. + DuplicateResourceLinkError: If the link already exists. + DatabaseError: On transient or unexpected DB errors. + """ + if parent_id == child_id: + raise CycleDetectedError(parent_id, child_id, [parent_id, child_id]) + + session = self._session() + try: + parent_row = ( + session.query(ResourceModel).filter_by(resource_id=parent_id).first() + ) + if parent_row is None: + raise ResourceNotFoundRepoError(parent_id) + + child_row = ( + session.query(ResourceModel).filter_by(resource_id=child_id).first() + ) + if child_row is None: + raise ResourceNotFoundRepoError(child_id) + + # Check type compatibility + parent_type_name = cast(str, parent_row.type_name) + child_type_name = cast(str, child_row.type_name) + + parent_type_row = ( + session.query(ResourceTypeModel) + .filter_by(name=parent_type_name) + .first() + ) + if parent_type_row is not None: + allowed_raw = cast( + "str | None", + parent_type_row.allowed_child_types_json, + ) + allowed_children: list[str] = ( + json.loads(allowed_raw) if allowed_raw else [] + ) + if allowed_children and child_type_name not in allowed_children: + raise TypeIncompatibleError(parent_type_name, child_type_name) + + # Check for duplicate link + existing = ( + session.query(ResourceLinkModel) + .filter_by(parent_id=parent_id, child_id=child_id) + .first() + ) + if existing is not None: + raise DuplicateResourceLinkError(parent_id, child_id) + + # Cycle detection: ensure child_id is not an + # ancestor of parent_id + ancestors = self._get_ancestors(session, parent_id) + if child_id in ancestors: + cycle_path = self._build_cycle_path(session, parent_id, child_id) + raise CycleDetectedError(parent_id, child_id, cycle_path) + + link = ResourceLinkModel( + parent_id=parent_id, + child_id=child_id, + created_at=datetime.now(tz=UTC).isoformat(), + ) + session.add(link) + session.flush() + except ( + ResourceNotFoundRepoError, + TypeIncompatibleError, + CycleDetectedError, + DuplicateResourceLinkError, + ): + raise + except IntegrityError as exc: + session.rollback() + raise DatabaseError( + f"Failed to link {parent_id} -> {child_id}: {exc}" + ) from exc + except ( + OperationalError, + SQLAlchemyDatabaseError, + ) as exc: + session.rollback() + raise DatabaseError( + f"Failed to link {parent_id} -> {child_id}: {exc}" + ) from exc + + @database_retry + def unlink_child(self, parent_id: str, child_id: str) -> None: + """Remove a parent-child link from the DAG. + + Args: + parent_id: ULID of the parent resource. + child_id: ULID of the child resource. + + Raises: + ResourceNotFoundRepoError: If either resource missing. + LinkNotFoundError: If the link does not exist. + DatabaseError: On transient or unexpected DB errors. + """ + session = self._session() + try: + parent_row = ( + session.query(ResourceModel).filter_by(resource_id=parent_id).first() + ) + if parent_row is None: + raise ResourceNotFoundRepoError(parent_id) + + child_row = ( + session.query(ResourceModel).filter_by(resource_id=child_id).first() + ) + if child_row is None: + raise ResourceNotFoundRepoError(child_id) + + link = ( + session.query(ResourceLinkModel) + .filter_by(parent_id=parent_id, child_id=child_id) + .first() + ) + if link is None: + raise LinkNotFoundError(parent_id, child_id) + + session.delete(link) + session.flush() + except ( + ResourceNotFoundRepoError, + LinkNotFoundError, + ): + raise + except ( + OperationalError, + SQLAlchemyDatabaseError, + ) as exc: + session.rollback() + raise DatabaseError( + f"Failed to unlink {parent_id} -> {child_id}: {exc}" + ) from exc + + @database_retry + def get_children(self, resource_id: str) -> list[Any]: + """Get all direct children of a resource. + + Args: + resource_id: ULID of the parent resource. + + Returns: + List of child ``Resource`` domain objects. + """ + session = self._session() + try: + links = ( + session.query(ResourceLinkModel).filter_by(parent_id=resource_id).all() + ) + children: list[Any] = [] + for link in links: + child_row = ( + session.query(ResourceModel) + .filter_by(resource_id=cast(str, link.child_id)) + .first() + ) + if child_row is not None: + children.append(self._to_domain(child_row)) + return children + except ( + OperationalError, + SQLAlchemyDatabaseError, + ) as exc: + raise DatabaseError( + f"Failed to get children of '{resource_id}': {exc}" + ) from exc + + @database_retry + def get_parents(self, resource_id: str) -> list[Any]: + """Get all direct parents of a resource. + + Args: + resource_id: ULID of the child resource. + + Returns: + List of parent ``Resource`` domain objects. + """ + session = self._session() + try: + links = ( + session.query(ResourceLinkModel).filter_by(child_id=resource_id).all() + ) + parents: list[Any] = [] + for link in links: + parent_row = ( + session.query(ResourceModel) + .filter_by(resource_id=cast(str, link.parent_id)) + .first() + ) + if parent_row is not None: + parents.append(self._to_domain(parent_row)) + return parents + except ( + OperationalError, + SQLAlchemyDatabaseError, + ) as exc: + raise DatabaseError( + f"Failed to get parents of '{resource_id}': {exc}" + ) from exc + + @database_retry + def auto_discover_children(self, resource_id: str) -> list[Any]: + """Materialize child resources per type auto-discovery. + + Looks up the resource's type, checks auto_discovery config, + and for each child type with auto-discover enabled, creates + a child resource and links it to the parent. + + Args: + resource_id: ULID of the parent resource. + + Returns: + List of newly created child ``Resource`` domain objects. + + Raises: + ResourceNotFoundRepoError: If the resource is missing. + DatabaseError: On transient or unexpected DB errors. + """ + from ulid import ULID as _ULID + + from cleveragents.domain.models.core.resource import ( + PhysVirt, + Resource, + ResourceCapabilities, + ) + + session = self._session() + try: + parent_row = ( + session.query(ResourceModel).filter_by(resource_id=resource_id).first() + ) + if parent_row is None: + raise ResourceNotFoundRepoError(resource_id) + + parent_type_name = cast(str, parent_row.type_name) + type_row = ( + session.query(ResourceTypeModel) + .filter_by(name=parent_type_name) + .first() + ) + if type_row is None: + return [] + + # Parse auto_discovery config + auto_disc_raw = cast("str | None", type_row.auto_discover_json) + if not auto_disc_raw: + return [] + + auto_disc: dict[str, Any] = json.loads(auto_disc_raw) + if not auto_disc.get("enabled", False): + return [] + + rules: list[dict[str, Any]] = auto_disc.get("rules", []) + if not rules: + return [] + + # Parse allowed child types + child_types_raw = cast( + "str | None", + type_row.allowed_child_types_json, + ) + allowed_child_types: list[str] = ( + json.loads(child_types_raw) if child_types_raw else [] + ) + + created: list[Any] = [] + for rule in rules: + child_type_name = rule.get("type", "") + if not child_type_name: + continue + + # Verify child type exists in DB + ct_row = ( + session.query(ResourceTypeModel) + .filter_by(name=child_type_name) + .first() + ) + if ct_row is None: + continue + + # Verify type compatibility + if allowed_child_types and child_type_name not in allowed_child_types: + continue + + ct_kind = cast(str, ct_row.resource_kind) + + now_iso = datetime.now(tz=UTC).isoformat() + child_id = str(_ULID()) + child_model = ResourceModel( + resource_id=child_id, + namespaced_name=None, + namespace=None, + type_name=child_type_name, + resource_kind=ct_kind, + location=None, + description=(f"Auto-discovered {child_type_name}"), + read_only=False, + auto_discovered=True, + sandbox_strategy=None, + content_hash=None, + properties_json=None, + metadata_json=None, + created_at=now_iso, + updated_at=now_iso, + ) + session.add(child_model) + session.flush() + + # Link child to parent + link = ResourceLinkModel( + parent_id=resource_id, + child_id=child_id, + created_at=now_iso, + ) + session.add(link) + session.flush() + + child_resource = Resource( + resource_id=child_id, + name=None, + resource_type_name=child_type_name, + classification=PhysVirt(ct_kind), + description=(f"Auto-discovered {child_type_name}"), + properties={}, + location=None, + content_hash=None, + sandbox_strategy=None, + capabilities=ResourceCapabilities(), + created_at=datetime.fromisoformat(now_iso), + updated_at=datetime.fromisoformat(now_iso), + ) + created.append(child_resource) + + return created + except ResourceNotFoundRepoError: + raise + except ( + OperationalError, + SQLAlchemyDatabaseError, + ) as exc: + session.rollback() + raise DatabaseError( + f"Failed to auto-discover children for '{resource_id}': {exc}" + ) from exc + + @staticmethod + def _get_ancestors(session: Session, resource_id: str) -> set[str]: + """Return all ancestor resource IDs (BFS upward). + + Used for cycle detection: if a proposed child is + already an ancestor of the parent, linking would + create a cycle. + """ + visited: set[str] = set() + queue: list[str] = [resource_id] + while queue: + current = queue.pop(0) + if current in visited: + continue + visited.add(current) + parent_links = ( + session.query(ResourceLinkModel).filter_by(child_id=current).all() + ) + for link in parent_links: + pid = cast(str, link.parent_id) + if pid not in visited: + queue.append(pid) + return visited + + @staticmethod + def _build_cycle_path( + session: Session, + parent_id: str, + child_id: str, + ) -> list[str]: + """Build a path showing the cycle for error msgs. + + Returns a list like [child_id, ..., parent_id, + child_id] showing the cycle. + """ + # BFS from child_id upward to find parent_id + predecessors: dict[str, str | None] = {parent_id: None} + queue: list[str] = [parent_id] + found = False + while queue and not found: + current = queue.pop(0) + parent_links = ( + session.query(ResourceLinkModel).filter_by(child_id=current).all() + ) + for link in parent_links: + pid = cast(str, link.parent_id) + if pid not in predecessors: + predecessors[pid] = current + if pid == child_id: + found = True + break + queue.append(pid) + + # Reconstruct path + path: list[str] = [] + current_node: str | None = child_id + while current_node is not None: + path.append(current_node) + current_node = predecessors.get(current_node) + path.append(child_id) + return path + @database_retry def resolve_namespaced_name(self, name_or_id: str) -> Any | None: """Resolve a resource by namespaced name first, then try ULID. @@ -2639,6 +3106,22 @@ class DuplicateToolError(DatabaseError): self.tool_name = name +class ToolNotFoundError(DatabaseError): + """Raised when a requested tool is not found.""" + + def __init__(self, name: str): + super().__init__(f"Tool '{name}' not found") + self.tool_name = name + + +class InvalidToolTypeError(BusinessRuleViolation): + """Raised when a tool has an unsupported tool_type value.""" + + def __init__(self, tool_type: str): + super().__init__(f"Invalid tool_type '{tool_type}'") + self.tool_type = tool_type + + class ToolInUseError(BusinessRuleViolation): """Raised when deleting a tool that still has validation attachments.""" @@ -2651,34 +3134,29 @@ class ToolInUseError(BusinessRuleViolation): self.attachment_count = attachment_count -class ToolNotFoundError(DatabaseError): - """Raised when a tool cannot be found for update or retrieval.""" - - def __init__(self, name: str): - super().__init__(f"Tool '{name}' not found") - self.tool_name = name - - -class InvalidToolTypeError(DatabaseError): - """Raised when creating a tool with an unsupported tool_type.""" - - _VALID: ClassVar[set[str]] = {"tool", "validation"} - - def __init__(self, tool_type: str): - super().__init__( - f"Invalid tool_type '{tool_type}': must be one of {sorted(self._VALID)}" - ) - self.tool_type = tool_type - - class DuplicateValidationAttachmentError(DatabaseError): - """Raised when a duplicate validation attachment is created.""" + """Raised when attaching a validation that already exists.""" - def __init__(self, detail: str = ""): - msg = "Duplicate validation attachment" - if detail: - msg = f"{msg}: {detail}" - super().__init__(msg) + def __init__( + self, + validation_name: str, + resource_id: str, + project_name: str | None = None, + plan_id: str | None = None, + ) -> None: + detail = ( + f"validation '{validation_name}' already attached to resource " + f"'{resource_id}'" + ) + if project_name is not None: + detail += f" for project '{project_name}'" + if plan_id is not None: + detail += f" and plan '{plan_id}'" + super().__init__(detail) + self.validation_name = validation_name + self.resource_id = resource_id + self.project_name = project_name + self.plan_id = plan_id class ToolRegistryRepository: @@ -2806,7 +3284,7 @@ class ToolRegistryRepository: try: row = session.query(ToolModel).filter_by(name=name_str).first() if row is None: - raise ToolNotFoundError(name_str) + raise DatabaseError(f"Tool '{name_str}' not found for update") from datetime import datetime as _dt @@ -2897,8 +3375,7 @@ class ToolRepository(ToolRegistryRepository): """Compatibility wrapper around ``ToolRegistryRepository``. Provides the legacy ``add / get / remove`` method names expected by - Jeff's test suites while delegating to the canonical repository - implementation underneath. + older test suites while delegating to the canonical repository. """ _VALID_TOOL_TYPES: ClassVar[set[str]] = {"tool", "validation"} @@ -2933,7 +3410,6 @@ class ToolRepository(ToolRegistryRepository): if not isinstance(obj, dict) else obj.get(key, default) ) - # Unwrap enum / SimpleNamespace wrapper with explicit .value if raw is not None and "value" in raw.__class__.__dict__: return raw.value if ( @@ -2942,7 +3418,6 @@ class ToolRepository(ToolRegistryRepository): and "value" in getattr(raw, "__dict__", {}) ): return raw.value - # Reject non-scalar types (e.g. MagicMock auto-attributes) if not isinstance(raw, _SCALAR_TYPES): return default return raw @@ -3010,36 +3485,76 @@ class ToolRepository(ToolRegistryRepository): # -- public API -------------------------------------------------------- def add(self, tool: Any) -> str: - """Persist a tool and return the tool name (identifier). - - Validates ``tool_type`` before delegating to - ``ToolRegistryRepository.create``. - - Raises: - InvalidToolTypeError: If tool_type is not in the valid set. - """ + """Persist a tool and return the tool name (identifier).""" tool_type = self._extract_value(tool, "tool_type", "tool") if tool_type not in self._VALID_TOOL_TYPES: raise InvalidToolTypeError(str(tool_type)) prepared = self._prepare_tool_dict(tool) - self.create(prepared) + try: + self.create(prepared) + except DuplicateToolError: + raise + except DatabaseError as exc: + raise DatabaseError(f"Failed to add tool: {exc}") from exc return prepared["name"] def get(self, tool_id: str) -> Any | None: - """Retrieve a tool by identifier (delegates to ``get_by_name``).""" - return self.get_by_name(tool_id) + session = self._session() + try: + row = session.query(ToolModel).filter_by(name=tool_id).first() + if row is None: + return None + return self._to_legacy_domain(row) + except (OperationalError, SQLAlchemyDatabaseError) as exc: + raise DatabaseError(f"Failed to get tool {tool_id}: {exc}") from exc + + def get_by_name(self, name: str) -> Any | None: + session = self._session() + try: + row = session.query(ToolModel).filter_by(name=name).first() + if row is None: + return None + return self._to_legacy_domain(row) + except (OperationalError, SQLAlchemyDatabaseError) as exc: + raise DatabaseError(f"Failed to get tool by name '{name}': {exc}") from exc def remove(self, name: str) -> bool: - """Remove a tool by name (delegates to ``delete``). + try: + deleted = self.delete(name) + except ToolInUseError: + raise + except DatabaseError as exc: + raise DatabaseError(f"Failed to remove tool {name}: {exc}") from exc - Raises: - ToolNotFoundError: If the tool does not exist. - """ - result = self.delete(name) - if not result: + if not deleted: raise ToolNotFoundError(name) - return result + return deleted + + def update(self, tool: Any) -> Any: + name_str = ( + tool.get("name", "") + if isinstance(tool, dict) + else getattr(tool, "name", "") + ) + try: + return super().update(tool) + except DatabaseError as exc: + if "not found" in str(exc).lower(): + raise ToolNotFoundError(name_str) from exc + raise + + @staticmethod + def _to_legacy_domain(row: ToolModel) -> Any: + from types import SimpleNamespace + + return SimpleNamespace( + name=cast(str, row.name), + input_schema=row.input_schema, + output_schema=row.output_schema, + tool_type=cast(str, row.tool_type), + source=cast(str, row.source), + ) class ValidationAttachmentRepository: @@ -3061,7 +3576,7 @@ class ValidationAttachmentRepository: self, validation_name: str, resource_id: str, - mode: str, + mode: str = "required", project_name: str | None = None, plan_id: str | None = None, args: dict[str, Any] | None = None, @@ -3083,6 +3598,9 @@ class ValidationAttachmentRepository: from ulid import ULID as _ULID + if "/" in resource_id and "/" not in validation_name: + validation_name, resource_id = resource_id, validation_name + session = self._session() try: # Check for existing attachment with same validation+resource+scope @@ -3103,10 +3621,12 @@ class ValidationAttachmentRepository: existing = ( session.query(ValidationAttachmentModel).filter(*dup_conditions).first() ) - if existing is not None: + if isinstance(existing, ValidationAttachmentModel): raise DuplicateValidationAttachmentError( - f"validation '{validation_name}' already attached " - f"to resource '{resource_id}'" + validation_name, + resource_id, + project_name=project_name, + plan_id=plan_id, ) now_iso = datetime.now().isoformat() @@ -3141,14 +3661,19 @@ class ValidationAttachmentRepository: except IntegrityError as exc: session.rollback() if "UNIQUE" in str(exc).upper() or "unique" in str(exc).lower(): - raise DuplicateValidationAttachmentError(str(exc)) from exc + raise DuplicateValidationAttachmentError( + validation_name, + resource_id, + project_name=project_name, + plan_id=plan_id, + ) from exc raise DatabaseError(f"Failed to attach validation: {exc}") from exc except (OperationalError, SQLAlchemyDatabaseError) as exc: session.rollback() raise DatabaseError(f"Failed to attach validation: {exc}") from exc @database_retry - def detach(self, attachment_id: str) -> bool: + def detach(self, attachment_id: str, validation_name: str | None = None) -> bool: """Remove a validation attachment by its ULID. Args: @@ -3157,6 +3682,7 @@ class ValidationAttachmentRepository: Returns: ``True`` if the attachment was removed. """ + _ = validation_name session = self._session() try: row = (