From b58c572a7dd50aafe4e1d115b84c98c32146d4d4 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sat, 7 Mar 2026 19:17:16 +0100 Subject: [PATCH 1/4] feat: implement hierarchical delegation and loop prevention (#12, #17) Add hierarchical delegation with authority validation, 5-mechanism loop prevention (ancestry, depth, dedup, rate limit, circuit breaker), and a DelegationService orchestrator with audit trail. - Task model: add parent_task_id and delegation_chain fields - HierarchyResolver: build org graph from Company, cycle detection - AuthorityValidator: chain-of-command + role permission checks - DelegationGuard: orchestrates all loop prevention mechanisms - DelegationService: end-to-end delegation with audit recording - 9 new error types for delegation and hierarchy failures - 14 delegation event constants for observability - Integration tests for multi-hop delegation flows Co-Authored-By: Claude Opus 4.6 --- src/ai_company/communication/__init__.py | 51 +++ .../communication/delegation/__init__.py | 27 ++ .../communication/delegation/authority.py | 144 +++++++ .../communication/delegation/hierarchy.py | 216 ++++++++++ .../communication/delegation/models.py | 102 +++++ .../communication/delegation/service.py | 195 +++++++++ src/ai_company/communication/errors.py | 36 ++ .../communication/loop_prevention/__init__.py | 35 ++ .../communication/loop_prevention/ancestry.py | 30 ++ .../loop_prevention/circuit_breaker.py | 154 +++++++ .../communication/loop_prevention/dedup.py | 92 ++++ .../communication/loop_prevention/depth.py | 30 ++ .../communication/loop_prevention/guard.py | 113 +++++ .../communication/loop_prevention/models.py | 26 ++ .../loop_prevention/rate_limit.py | 104 +++++ src/ai_company/core/task.py | 27 ++ .../observability/events/delegation.py | 23 + tests/integration/communication/__init__.py | 0 .../test_delegation_integration.py | 373 +++++++++++++++++ .../unit/communication/delegation/__init__.py | 0 .../delegation/test_authority.py | 180 ++++++++ .../delegation/test_hierarchy.py | 267 ++++++++++++ .../communication/delegation/test_models.py | 189 +++++++++ .../communication/delegation/test_service.py | 396 ++++++++++++++++++ .../communication/loop_prevention/__init__.py | 0 .../loop_prevention/test_ancestry.py | 43 ++ .../loop_prevention/test_circuit_breaker.py | 97 +++++ .../loop_prevention/test_dedup.py | 79 ++++ .../loop_prevention/test_depth.py | 46 ++ .../loop_prevention/test_guard.py | 117 ++++++ .../loop_prevention/test_rate_limit.py | 98 +++++ tests/unit/communication/test_errors.py | 45 ++ tests/unit/core/test_task.py | 57 +++ tests/unit/observability/test_events.py | 1 + 34 files changed, 3393 insertions(+) create mode 100644 src/ai_company/communication/delegation/__init__.py create mode 100644 src/ai_company/communication/delegation/authority.py create mode 100644 src/ai_company/communication/delegation/hierarchy.py create mode 100644 src/ai_company/communication/delegation/models.py create mode 100644 src/ai_company/communication/delegation/service.py create mode 100644 src/ai_company/communication/loop_prevention/__init__.py create mode 100644 src/ai_company/communication/loop_prevention/ancestry.py create mode 100644 src/ai_company/communication/loop_prevention/circuit_breaker.py create mode 100644 src/ai_company/communication/loop_prevention/dedup.py create mode 100644 src/ai_company/communication/loop_prevention/depth.py create mode 100644 src/ai_company/communication/loop_prevention/guard.py create mode 100644 src/ai_company/communication/loop_prevention/models.py create mode 100644 src/ai_company/communication/loop_prevention/rate_limit.py create mode 100644 src/ai_company/observability/events/delegation.py create mode 100644 tests/integration/communication/__init__.py create mode 100644 tests/integration/communication/test_delegation_integration.py create mode 100644 tests/unit/communication/delegation/__init__.py create mode 100644 tests/unit/communication/delegation/test_authority.py create mode 100644 tests/unit/communication/delegation/test_hierarchy.py create mode 100644 tests/unit/communication/delegation/test_models.py create mode 100644 tests/unit/communication/delegation/test_service.py create mode 100644 tests/unit/communication/loop_prevention/__init__.py create mode 100644 tests/unit/communication/loop_prevention/test_ancestry.py create mode 100644 tests/unit/communication/loop_prevention/test_circuit_breaker.py create mode 100644 tests/unit/communication/loop_prevention/test_dedup.py create mode 100644 tests/unit/communication/loop_prevention/test_depth.py create mode 100644 tests/unit/communication/loop_prevention/test_guard.py create mode 100644 tests/unit/communication/loop_prevention/test_rate_limit.py diff --git a/src/ai_company/communication/__init__.py b/src/ai_company/communication/__init__.py index cbe3a80ee0..276305fcbd 100644 --- a/src/ai_company/communication/__init__.py +++ b/src/ai_company/communication/__init__.py @@ -14,6 +14,15 @@ MessageRetentionConfig, RateLimitConfig, ) +from ai_company.communication.delegation import ( + AuthorityCheckResult, + AuthorityValidator, + DelegationRecord, + DelegationRequest, + DelegationResult, + DelegationService, + HierarchyResolver, +) from ai_company.communication.dispatcher import DispatchResult, MessageDispatcher from ai_company.communication.enums import ( AttachmentType, @@ -27,6 +36,15 @@ ChannelAlreadyExistsError, ChannelNotFoundError, CommunicationError, + DelegationAncestryError, + DelegationAuthorityError, + DelegationCircuitOpenError, + DelegationDepthError, + DelegationDuplicateError, + DelegationError, + DelegationLoopError, + DelegationRateLimitError, + HierarchyResolutionError, MessageBusAlreadyRunningError, MessageBusNotRunningError, NotSubscribedError, @@ -37,6 +55,16 @@ MessageHandler, MessageHandlerFunc, ) +from ai_company.communication.loop_prevention import ( + CircuitBreakerState, + DelegationCircuitBreaker, + DelegationDeduplicator, + DelegationGuard, + DelegationRateLimiter, + GuardCheckOutcome, + check_ancestry, + check_delegation_depth, +) from ai_company.communication.message import Attachment, Message, MessageMetadata from ai_company.communication.messenger import AgentMessenger from ai_company.communication.subscription import DeliveryEnvelope, Subscription @@ -45,19 +73,40 @@ "AgentMessenger", "Attachment", "AttachmentType", + "AuthorityCheckResult", + "AuthorityValidator", "Channel", "ChannelAlreadyExistsError", "ChannelNotFoundError", "ChannelType", "CircuitBreakerConfig", + "CircuitBreakerState", "CommunicationConfig", "CommunicationError", "CommunicationPattern", + "DelegationAncestryError", + "DelegationAuthorityError", + "DelegationCircuitBreaker", + "DelegationCircuitOpenError", + "DelegationDeduplicator", + "DelegationDepthError", + "DelegationDuplicateError", + "DelegationError", + "DelegationGuard", + "DelegationLoopError", + "DelegationRateLimiter", + "DelegationRecord", + "DelegationRequest", + "DelegationResult", + "DelegationService", "DeliveryEnvelope", "DispatchResult", "FunctionHandler", + "GuardCheckOutcome", "HandlerRegistration", "HierarchyConfig", + "HierarchyResolutionError", + "HierarchyResolver", "InMemoryMessageBus", "LoopPreventionConfig", "MeetingTypeConfig", @@ -78,4 +127,6 @@ "NotSubscribedError", "RateLimitConfig", "Subscription", + "check_ancestry", + "check_delegation_depth", ] diff --git a/src/ai_company/communication/delegation/__init__.py b/src/ai_company/communication/delegation/__init__.py new file mode 100644 index 0000000000..8fffae890b --- /dev/null +++ b/src/ai_company/communication/delegation/__init__.py @@ -0,0 +1,27 @@ +"""Hierarchical delegation subsystem.""" + +from ai_company.communication.delegation.authority import ( + AuthorityCheckResult, + AuthorityValidator, +) +from ai_company.communication.delegation.hierarchy import ( + HierarchyResolver, +) +from ai_company.communication.delegation.models import ( + DelegationRecord, + DelegationRequest, + DelegationResult, +) +from ai_company.communication.delegation.service import ( + DelegationService, +) + +__all__ = [ + "AuthorityCheckResult", + "AuthorityValidator", + "DelegationRecord", + "DelegationRequest", + "DelegationResult", + "DelegationService", + "HierarchyResolver", +] diff --git a/src/ai_company/communication/delegation/authority.py b/src/ai_company/communication/delegation/authority.py new file mode 100644 index 0000000000..73ff6fcd87 --- /dev/null +++ b/src/ai_company/communication/delegation/authority.py @@ -0,0 +1,144 @@ +"""Authority validation for hierarchical delegation.""" + +from pydantic import BaseModel, ConfigDict, Field + +from ai_company.communication.config import HierarchyConfig # noqa: TC001 +from ai_company.communication.delegation.hierarchy import ( # noqa: TC001 + HierarchyResolver, +) +from ai_company.core.agent import AgentIdentity # noqa: TC001 +from ai_company.observability import get_logger +from ai_company.observability.events.delegation import ( + DELEGATION_AUTHORITY_DENIED, + DELEGATION_AUTHORIZED, +) + +logger = get_logger(__name__) + + +class AuthorityCheckResult(BaseModel): + """Result of an authority validation check. + + Attributes: + allowed: Whether the delegation is authorized. + reason: Explanation (empty on success). + """ + + model_config = ConfigDict(frozen=True) + + allowed: bool = Field(description="Whether delegation is allowed") + reason: str = Field(default="", description="Explanation") + + +class AuthorityValidator: + """Validates delegation authority using hierarchy and role permissions. + + Checks: + 1. Hierarchy: delegatee must be a subordinate of delegator + (direct or skip-level depending on config). + 2. Roles: if ``delegator.authority.can_delegate_to`` is + non-empty, ``delegatee.role`` must be in it. + + Args: + hierarchy: Resolved org hierarchy. + hierarchy_config: Hierarchy enforcement configuration. + """ + + __slots__ = ("_config", "_hierarchy") + + def __init__( + self, + hierarchy: HierarchyResolver, + hierarchy_config: HierarchyConfig, + ) -> None: + self._hierarchy = hierarchy + self._config = hierarchy_config + + def validate( + self, + delegator: AgentIdentity, + delegatee: AgentIdentity, + ) -> AuthorityCheckResult: + """Validate whether delegator can delegate to delegatee. + + Args: + delegator: Identity of the delegating agent. + delegatee: Identity of the target agent. + + Returns: + Result indicating whether delegation is authorized. + """ + if self._config.enforce_chain_of_command: + result = self._check_hierarchy(delegator, delegatee) + if not result.allowed: + return result + + result = self._check_role_permissions(delegator, delegatee) + if not result.allowed: + return result + + logger.debug( + DELEGATION_AUTHORIZED, + delegator=delegator.name, + delegatee=delegatee.name, + ) + return AuthorityCheckResult(allowed=True) + + def _check_hierarchy( + self, + delegator: AgentIdentity, + delegatee: AgentIdentity, + ) -> AuthorityCheckResult: + """Check hierarchy constraints.""" + is_direct = self._hierarchy.is_direct_report(delegator.name, delegatee.name) + if is_direct: + return AuthorityCheckResult(allowed=True) + + if self._config.allow_skip_level: + is_sub = self._hierarchy.is_subordinate(delegator.name, delegatee.name) + if is_sub: + return AuthorityCheckResult(allowed=True) + + reason = ( + f"{delegatee.name!r} is not a " + f"{'subordinate' if self._config.allow_skip_level else 'direct report'} " + f"of {delegator.name!r}" + ) + logger.info( + DELEGATION_AUTHORITY_DENIED, + delegator=delegator.name, + delegatee=delegatee.name, + reason=reason, + ) + return AuthorityCheckResult( + allowed=False, + reason=reason, + ) + + def _check_role_permissions( + self, + delegator: AgentIdentity, + delegatee: AgentIdentity, + ) -> AuthorityCheckResult: + """Check role-based delegation permissions.""" + allowed_roles = delegator.authority.can_delegate_to + if not allowed_roles: + return AuthorityCheckResult(allowed=True) + + if delegatee.role in allowed_roles: + return AuthorityCheckResult(allowed=True) + + reason = ( + f"Role {delegatee.role!r} is not in " + f"delegator's can_delegate_to: {allowed_roles}" + ) + logger.info( + DELEGATION_AUTHORITY_DENIED, + delegator=delegator.name, + delegatee=delegatee.name, + reason=reason, + ) + return AuthorityCheckResult( + allowed=False, + reason=reason, + ) diff --git a/src/ai_company/communication/delegation/hierarchy.py b/src/ai_company/communication/delegation/hierarchy.py new file mode 100644 index 0000000000..3a6fc44957 --- /dev/null +++ b/src/ai_company/communication/delegation/hierarchy.py @@ -0,0 +1,216 @@ +"""Hierarchy resolver for organizational structure.""" + +from ai_company.communication.errors import HierarchyResolutionError +from ai_company.core.company import Company # noqa: TC001 +from ai_company.observability import get_logger +from ai_company.observability.events.delegation import ( + DELEGATION_HIERARCHY_BUILT, + DELEGATION_HIERARCHY_CYCLE, +) + +logger = get_logger(__name__) + + +class HierarchyResolver: + """Resolves org hierarchy from a Company structure (read-only). + + Built from three sources, in priority order: + + 1. Explicit ``ReportingLine.supervisor`` (most specific) + 2. ``Team.lead`` for team members + 3. ``Department.head`` for team leads without explicit reporting + + Detects cycles at construction time. + + Args: + company: Frozen company structure to resolve hierarchy from. + + Raises: + HierarchyResolutionError: If a cycle is detected. + """ + + __slots__ = ("_reports_of", "_supervisor_of") + + def __init__(self, company: Company) -> None: + supervisor_of: dict[str, str] = {} + reports_of: dict[str, list[str]] = {} + + for dept in company.departments: + for team in dept.teams: + # Team lead → department head (lowest priority) + if team.lead not in supervisor_of: + supervisor_of[team.lead] = dept.head + reports_of.setdefault(dept.head, []).append(team.lead) + + # Team members → team lead (medium priority) + for member in team.members: + if member == team.lead: + continue + if member not in supervisor_of: + supervisor_of[member] = team.lead + reports_of.setdefault(team.lead, []).append(member) + + # Explicit reporting lines (highest priority — override) + for line in dept.reporting_lines: + old_sup = supervisor_of.get(line.subordinate) + if old_sup is not None and old_sup != line.supervisor: + # Remove from old supervisor's reports + old_reports = reports_of.get(old_sup, []) + if line.subordinate in old_reports: + old_reports.remove(line.subordinate) + supervisor_of[line.subordinate] = line.supervisor + reports_of.setdefault(line.supervisor, []).append(line.subordinate) + + # Cycle detection + self._detect_cycles(supervisor_of) + + # Freeze internal state + self._supervisor_of: dict[str, str] = supervisor_of + self._reports_of: dict[str, tuple[str, ...]] = { + k: tuple(v) for k, v in reports_of.items() + } + + logger.debug( + DELEGATION_HIERARCHY_BUILT, + agents=len(supervisor_of), + supervisors=len(reports_of), + ) + + @staticmethod + def _detect_cycles(supervisor_of: dict[str, str]) -> None: + """Detect cycles in the supervisor graph. + + Args: + supervisor_of: Mapping from agent to supervisor. + + Raises: + HierarchyResolutionError: If a cycle is found. + """ + for agent in supervisor_of: + visited: set[str] = set() + current = agent + while current in supervisor_of: + if current in visited: + logger.warning( + DELEGATION_HIERARCHY_CYCLE, + agent=agent, + cycle_at=current, + ) + msg = ( + f"Cycle detected in hierarchy at " + f"{current!r} (starting from {agent!r})" + ) + raise HierarchyResolutionError( + msg, + context={ + "agent": agent, + "cycle_at": current, + }, + ) + visited.add(current) + current = supervisor_of[current] + + def get_supervisor(self, agent_name: str) -> str | None: + """Get the direct supervisor of an agent. + + Args: + agent_name: Agent name to look up. + + Returns: + Supervisor name or None if the agent is at the top. + """ + return self._supervisor_of.get(agent_name) + + def get_direct_reports( + self, + agent_name: str, + ) -> tuple[str, ...]: + """Get all direct reports of an agent. + + Args: + agent_name: Supervisor agent name. + + Returns: + Tuple of direct report agent names. + """ + return self._reports_of.get(agent_name, ()) + + def is_direct_report( + self, + supervisor: str, + subordinate: str, + ) -> bool: + """Check if subordinate directly reports to supervisor. + + Args: + supervisor: Supervisor agent name. + subordinate: Potential subordinate agent name. + + Returns: + True if subordinate is a direct report. + """ + return subordinate in self.get_direct_reports(supervisor) + + def is_subordinate( + self, + supervisor: str, + subordinate: str, + ) -> bool: + """Check if subordinate is anywhere below supervisor. + + Walks up the hierarchy from subordinate to root. + + Args: + supervisor: Supervisor agent name. + subordinate: Potential subordinate agent name. + + Returns: + True if subordinate is below supervisor at any depth. + """ + current = subordinate + while current in self._supervisor_of: + current = self._supervisor_of[current] + if current == supervisor: + return True + return False + + def get_ancestors(self, agent_name: str) -> tuple[str, ...]: + """Get all ancestors from agent up to root. + + Args: + agent_name: Agent to start from. + + Returns: + Tuple of ancestor names, bottom-up (immediate supervisor + first, root last). + """ + ancestors: list[str] = [] + current = agent_name + while current in self._supervisor_of: + current = self._supervisor_of[current] + ancestors.append(current) + return tuple(ancestors) + + def get_delegation_depth( + self, + from_agent: str, + to_agent: str, + ) -> int | None: + """Get hierarchy levels between two agents. + + Args: + from_agent: Higher-level agent (potential supervisor). + to_agent: Lower-level agent (potential subordinate). + + Returns: + Number of hierarchy levels between them, or None if + to_agent is not below from_agent. + """ + depth = 0 + current = to_agent + while current in self._supervisor_of: + current = self._supervisor_of[current] + depth += 1 + if current == from_agent: + return depth + return None diff --git a/src/ai_company/communication/delegation/models.py b/src/ai_company/communication/delegation/models.py new file mode 100644 index 0000000000..e4da3a5053 --- /dev/null +++ b/src/ai_company/communication/delegation/models.py @@ -0,0 +1,102 @@ +"""Delegation request, result, and audit trail models.""" + +from pydantic import AwareDatetime, BaseModel, ConfigDict, Field + +from ai_company.core.task import Task # noqa: TC001 +from ai_company.core.types import NotBlankStr # noqa: TC001 + + +class DelegationRequest(BaseModel): + """Request to delegate a task down the hierarchy. + + Attributes: + delegator_id: Agent ID of the delegator. + delegatee_id: Agent ID of the target agent. + task: The task to delegate. + refinement: Additional context from the delegator. + constraints: Extra constraints for the delegatee. + """ + + model_config = ConfigDict(frozen=True) + + delegator_id: NotBlankStr = Field( + description="Agent ID of the delegator", + ) + delegatee_id: NotBlankStr = Field( + description="Agent ID of the target agent", + ) + task: Task = Field(description="Task to delegate") + refinement: str = Field( + default="", + description="Additional context from the delegator", + ) + constraints: tuple[NotBlankStr, ...] = Field( + default=(), + description="Extra constraints for the delegatee", + ) + + +class DelegationResult(BaseModel): + """Outcome of a delegation attempt. + + Attributes: + success: Whether the delegation succeeded. + delegated_task: The sub-task created, if successful. + rejection_reason: Reason for rejection, if unsuccessful. + blocked_by: Mechanism name that blocked, if applicable. + """ + + model_config = ConfigDict(frozen=True) + + success: bool = Field(description="Whether delegation succeeded") + delegated_task: Task | None = Field( + default=None, + description="Sub-task created on success", + ) + rejection_reason: str | None = Field( + default=None, + description="Reason for rejection", + ) + blocked_by: NotBlankStr | None = Field( + default=None, + description="Mechanism name that blocked delegation", + ) + + +class DelegationRecord(BaseModel): + """Audit trail entry for a completed delegation. + + Attributes: + delegation_id: Unique delegation identifier. + delegator_id: Agent ID of the delegator. + delegatee_id: Agent ID of the delegatee. + original_task_id: ID of the original task. + delegated_task_id: ID of the created sub-task. + timestamp: When the delegation occurred. + refinement: Context provided by the delegator. + """ + + model_config = ConfigDict(frozen=True) + + delegation_id: NotBlankStr = Field( + description="Unique delegation identifier", + ) + delegator_id: NotBlankStr = Field( + description="Delegator agent ID", + ) + delegatee_id: NotBlankStr = Field( + description="Delegatee agent ID", + ) + original_task_id: NotBlankStr = Field( + description="Original task ID", + ) + delegated_task_id: NotBlankStr = Field( + description="Created sub-task ID", + ) + timestamp: AwareDatetime = Field( + description="When delegation occurred", + ) + refinement: str = Field( + default="", + description="Context provided by delegator", + ) diff --git a/src/ai_company/communication/delegation/service.py b/src/ai_company/communication/delegation/service.py new file mode 100644 index 0000000000..abc325fd23 --- /dev/null +++ b/src/ai_company/communication/delegation/service.py @@ -0,0 +1,195 @@ +"""Delegation service orchestrating hierarchy, authority, and loop prevention.""" + +from datetime import UTC, datetime +from uuid import uuid4 + +from ai_company.communication.delegation.authority import ( # noqa: TC001 + AuthorityValidator, +) +from ai_company.communication.delegation.hierarchy import ( # noqa: TC001 + HierarchyResolver, +) +from ai_company.communication.delegation.models import ( + DelegationRecord, + DelegationRequest, + DelegationResult, +) +from ai_company.communication.loop_prevention.guard import ( # noqa: TC001 + DelegationGuard, +) +from ai_company.core.agent import AgentIdentity # noqa: TC001 +from ai_company.core.task import Task +from ai_company.observability import get_logger +from ai_company.observability.events.delegation import ( + DELEGATION_CREATED, + DELEGATION_REQUESTED, +) + +logger = get_logger(__name__) + + +class DelegationService: + """Orchestrates hierarchical delegation with loop prevention. + + Validates authority, checks loop prevention guards, creates + sub-tasks, and records audit trail entries. The core logic is + synchronous (CPU-only); messaging is a separate async concern. + + Args: + hierarchy: Resolved organizational hierarchy. + authority_validator: Authority validation logic. + guard: Loop prevention guard. + """ + + __slots__ = ( + "_audit_trail", + "_authority_validator", + "_guard", + "_hierarchy", + ) + + def __init__( + self, + *, + hierarchy: HierarchyResolver, + authority_validator: AuthorityValidator, + guard: DelegationGuard, + ) -> None: + self._hierarchy = hierarchy + self._authority_validator = authority_validator + self._guard = guard + self._audit_trail: list[DelegationRecord] = [] + + def delegate( + self, + request: DelegationRequest, + delegator: AgentIdentity, + delegatee: AgentIdentity, + ) -> DelegationResult: + """Execute a delegation: authority, loops, sub-task, audit. + + Args: + request: The delegation request. + delegator: Identity of the delegating agent. + delegatee: Identity of the target agent. + + Returns: + Result indicating success or rejection with reason. + """ + logger.info( + DELEGATION_REQUESTED, + delegator=request.delegator_id, + delegatee=request.delegatee_id, + task_id=request.task.id, + ) + + # 1. Authority check + auth_result = self._authority_validator.validate(delegator, delegatee) + if not auth_result.allowed: + return DelegationResult( + success=False, + rejection_reason=auth_result.reason, + blocked_by="authority", + ) + + # 2. Loop prevention checks + guard_outcome = self._guard.check( + delegation_chain=request.task.delegation_chain, + delegator_id=request.delegator_id, + delegatee_id=request.delegatee_id, + task_title=request.task.title, + ) + if not guard_outcome.passed: + return DelegationResult( + success=False, + rejection_reason=guard_outcome.message, + blocked_by=guard_outcome.mechanism, + ) + + # 3. Create sub-task + sub_task = self.create_sub_task(request) + + # 4. Record in guard and audit trail + self._guard.record_delegation( + request.delegator_id, + request.delegatee_id, + request.task.title, + ) + record = DelegationRecord( + delegation_id=str(uuid4()), + delegator_id=request.delegator_id, + delegatee_id=request.delegatee_id, + original_task_id=request.task.id, + delegated_task_id=sub_task.id, + timestamp=datetime.now(UTC), + refinement=request.refinement, + ) + self._audit_trail.append(record) + + logger.info( + DELEGATION_CREATED, + delegator=request.delegator_id, + delegatee=request.delegatee_id, + original_task_id=request.task.id, + delegated_task_id=sub_task.id, + ) + + return DelegationResult( + success=True, + delegated_task=sub_task, + ) + + def create_sub_task(self, request: DelegationRequest) -> Task: + """Create a new sub-task from a delegation request. + + The sub-task inherits the original task's properties but gets + a new ID, parent reference, extended delegation chain, and + CREATED status. + + Args: + request: The delegation request. + + Returns: + New Task with delegation metadata. + """ + original = request.task + new_chain = (*original.delegation_chain, request.delegator_id) + description = original.description + if request.refinement: + description = ( + f"{original.description}\n\nDelegation context: {request.refinement}" + ) + + return Task( + id=f"del-{uuid4().hex[:12]}", + title=original.title, + description=description, + type=original.type, + priority=original.priority, + project=original.project, + created_by=request.delegator_id, + parent_task_id=original.id, + delegation_chain=new_chain, + estimated_complexity=original.estimated_complexity, + budget_limit=original.budget_limit, + deadline=original.deadline, + ) + + def get_audit_trail(self) -> tuple[DelegationRecord, ...]: + """Return all delegation audit records. + + Returns: + Tuple of delegation records in chronological order. + """ + return tuple(self._audit_trail) + + def get_supervisor_of(self, agent_name: str) -> str | None: + """Expose hierarchy lookup for escalation callers. + + Args: + agent_name: Agent name to look up. + + Returns: + Supervisor name or None if at the top. + """ + return self._hierarchy.get_supervisor(agent_name) diff --git a/src/ai_company/communication/errors.py b/src/ai_company/communication/errors.py index 501e7b0146..96ccac0104 100644 --- a/src/ai_company/communication/errors.py +++ b/src/ai_company/communication/errors.py @@ -62,3 +62,39 @@ class MessageBusNotRunningError(CommunicationError): class MessageBusAlreadyRunningError(CommunicationError): """start() called on a message bus that is already running.""" + + +class DelegationError(CommunicationError): + """Base exception for delegation-related errors.""" + + +class DelegationAuthorityError(DelegationError): + """Delegator lacks authority to delegate to the target agent.""" + + +class DelegationLoopError(DelegationError): + """Base for loop prevention mechanism rejections.""" + + +class DelegationDepthError(DelegationLoopError): + """Delegation chain exceeds maximum depth.""" + + +class DelegationAncestryError(DelegationLoopError): + """Delegation would create a cycle in the task ancestry.""" + + +class DelegationRateLimitError(DelegationLoopError): + """Delegation rate limit exceeded for agent pair.""" + + +class DelegationCircuitOpenError(DelegationLoopError): + """Circuit breaker is open for agent pair.""" + + +class DelegationDuplicateError(DelegationLoopError): + """Duplicate delegation detected within dedup window.""" + + +class HierarchyResolutionError(CommunicationError): + """Error resolving organizational hierarchy.""" diff --git a/src/ai_company/communication/loop_prevention/__init__.py b/src/ai_company/communication/loop_prevention/__init__.py new file mode 100644 index 0000000000..9f3bb6f6ba --- /dev/null +++ b/src/ai_company/communication/loop_prevention/__init__.py @@ -0,0 +1,35 @@ +"""Loop prevention mechanisms for delegation safety.""" + +from ai_company.communication.loop_prevention.ancestry import ( + check_ancestry, +) +from ai_company.communication.loop_prevention.circuit_breaker import ( + CircuitBreakerState, + DelegationCircuitBreaker, +) +from ai_company.communication.loop_prevention.dedup import ( + DelegationDeduplicator, +) +from ai_company.communication.loop_prevention.depth import ( + check_delegation_depth, +) +from ai_company.communication.loop_prevention.guard import ( + DelegationGuard, +) +from ai_company.communication.loop_prevention.models import ( + GuardCheckOutcome, +) +from ai_company.communication.loop_prevention.rate_limit import ( + DelegationRateLimiter, +) + +__all__ = [ + "CircuitBreakerState", + "DelegationCircuitBreaker", + "DelegationDeduplicator", + "DelegationGuard", + "DelegationRateLimiter", + "GuardCheckOutcome", + "check_ancestry", + "check_delegation_depth", +] diff --git a/src/ai_company/communication/loop_prevention/ancestry.py b/src/ai_company/communication/loop_prevention/ancestry.py new file mode 100644 index 0000000000..5157ab6981 --- /dev/null +++ b/src/ai_company/communication/loop_prevention/ancestry.py @@ -0,0 +1,30 @@ +"""Ancestry cycle detection check (pure function).""" + +from ai_company.communication.loop_prevention.models import GuardCheckOutcome + +_MECHANISM = "ancestry" + + +def check_ancestry( + delegation_chain: tuple[str, ...], + delegatee_id: str, +) -> GuardCheckOutcome: + """Check whether delegating to delegatee would create an ancestry cycle. + + Args: + delegation_chain: Current chain of delegator agent IDs. + delegatee_id: Agent ID of the proposed delegatee. + + Returns: + Outcome with passed=False if delegatee is already in the chain. + """ + if delegatee_id in delegation_chain: + return GuardCheckOutcome( + passed=False, + mechanism=_MECHANISM, + message=( + f"Agent {delegatee_id!r} is already in the " + f"delegation chain: {delegation_chain}" + ), + ) + return GuardCheckOutcome(passed=True, mechanism=_MECHANISM) diff --git a/src/ai_company/communication/loop_prevention/circuit_breaker.py b/src/ai_company/communication/loop_prevention/circuit_breaker.py new file mode 100644 index 0000000000..834b075d47 --- /dev/null +++ b/src/ai_company/communication/loop_prevention/circuit_breaker.py @@ -0,0 +1,154 @@ +"""Circuit breaker for delegation bounces between agent pairs.""" + +import time +from collections.abc import Callable # noqa: TC003 +from enum import StrEnum + +from ai_company.communication.config import CircuitBreakerConfig # noqa: TC001 +from ai_company.communication.loop_prevention.models import GuardCheckOutcome +from ai_company.observability import get_logger +from ai_company.observability.events.delegation import ( + DELEGATION_LOOP_CIRCUIT_OPEN, +) + +logger = get_logger(__name__) + +_MECHANISM = "circuit_breaker" + + +class CircuitBreakerState(StrEnum): + """State of the circuit breaker for an agent pair. + + Members: + CLOSED: Normal operation, delegations allowed. + OPEN: Blocked, cooldown period active. + """ + + CLOSED = "closed" + OPEN = "open" + + +def _pair_key(a: str, b: str) -> tuple[str, str]: + """Create a canonical sorted key for an agent pair.""" + return (min(a, b), max(a, b)) + + +class _PairState: + """Internal mutable state for a single agent pair.""" + + __slots__ = ("bounce_count", "opened_at") + + def __init__(self) -> None: + self.bounce_count: int = 0 + self.opened_at: float | None = None + + +class DelegationCircuitBreaker: + """Tracks delegation bounces per sorted agent pair. + + After ``bounce_threshold`` bounces between the same pair, the + circuit opens for ``cooldown_seconds``. While open, all delegation + checks for that pair fail. + + Args: + config: Circuit breaker configuration. + clock: Monotonic clock function for deterministic testing. + """ + + __slots__ = ("_clock", "_config", "_pairs") + + def __init__( + self, + config: CircuitBreakerConfig, + *, + clock: Callable[[], float] = time.monotonic, + ) -> None: + self._config = config + self._clock = clock + self._pairs: dict[tuple[str, str], _PairState] = {} + + def _get_pair( + self, + delegator_id: str, + delegatee_id: str, + ) -> _PairState: + key = _pair_key(delegator_id, delegatee_id) + if key not in self._pairs: + self._pairs[key] = _PairState() + return self._pairs[key] + + def get_state( + self, + delegator_id: str, + delegatee_id: str, + ) -> CircuitBreakerState: + """Get the circuit breaker state for an agent pair. + + Args: + delegator_id: First agent ID. + delegatee_id: Second agent ID. + + Returns: + Current state of the circuit breaker. + """ + pair = self._get_pair(delegator_id, delegatee_id) + if pair.opened_at is not None: + elapsed = self._clock() - pair.opened_at + if elapsed < self._config.cooldown_seconds: + return CircuitBreakerState.OPEN + # Cooldown expired: reset + pair.bounce_count = 0 + pair.opened_at = None + return CircuitBreakerState.CLOSED + + def check( + self, + delegator_id: str, + delegatee_id: str, + ) -> GuardCheckOutcome: + """Check whether delegation is allowed for this pair. + + Args: + delegator_id: ID of the delegating agent. + delegatee_id: ID of the target agent. + + Returns: + Outcome with passed=False if circuit is open. + """ + state = self.get_state(delegator_id, delegatee_id) + if state is CircuitBreakerState.OPEN: + logger.info( + DELEGATION_LOOP_CIRCUIT_OPEN, + delegator=delegator_id, + delegatee=delegatee_id, + ) + return GuardCheckOutcome( + passed=False, + mechanism=_MECHANISM, + message=( + f"Circuit breaker open for pair " + f"({delegator_id!r}, {delegatee_id!r}); " + f"cooldown {self._config.cooldown_seconds}s" + ), + ) + return GuardCheckOutcome(passed=True, mechanism=_MECHANISM) + + def record_bounce( + self, + delegator_id: str, + delegatee_id: str, + ) -> None: + """Record a delegation bounce for the pair. + + If the bounce count reaches the threshold, opens the circuit. + + Args: + delegator_id: First agent ID. + delegatee_id: Second agent ID. + """ + pair = self._get_pair(delegator_id, delegatee_id) + # If cooldown expired, get_state already reset + self.get_state(delegator_id, delegatee_id) + pair.bounce_count += 1 + if pair.bounce_count >= self._config.bounce_threshold: + pair.opened_at = self._clock() diff --git a/src/ai_company/communication/loop_prevention/dedup.py b/src/ai_company/communication/loop_prevention/dedup.py new file mode 100644 index 0000000000..58122d6d43 --- /dev/null +++ b/src/ai_company/communication/loop_prevention/dedup.py @@ -0,0 +1,92 @@ +"""Delegation deduplication within a time window.""" + +import time +from collections.abc import Callable # noqa: TC003 + +from ai_company.communication.loop_prevention.models import GuardCheckOutcome +from ai_company.observability import get_logger +from ai_company.observability.events.delegation import ( + DELEGATION_LOOP_DEDUP_BLOCKED, +) + +logger = get_logger(__name__) + +_MECHANISM = "dedup" + + +class DelegationDeduplicator: + """Rejects duplicate (delegator, delegatee, task_title) within a window. + + Args: + window_seconds: Duration of the dedup window. + clock: Monotonic clock function for deterministic testing. + """ + + __slots__ = ("_clock", "_records", "_window_seconds") + + def __init__( + self, + window_seconds: int = 60, + *, + clock: Callable[[], float] = time.monotonic, + ) -> None: + self._window_seconds = window_seconds + self._clock = clock + self._records: dict[tuple[str, str, str], float] = {} + + def check( + self, + delegator_id: str, + delegatee_id: str, + task_title: str, + ) -> GuardCheckOutcome: + """Check for duplicate delegation within the window. + + Args: + delegator_id: ID of the delegating agent. + delegatee_id: ID of the target agent. + task_title: Title of the task being delegated. + + Returns: + Outcome with passed=False if a duplicate exists. + """ + key = (delegator_id, delegatee_id, task_title) + recorded_at = self._records.get(key) + if recorded_at is not None: + elapsed = self._clock() - recorded_at + if elapsed < self._window_seconds: + logger.info( + DELEGATION_LOOP_DEDUP_BLOCKED, + delegator=delegator_id, + delegatee=delegatee_id, + task_title=task_title, + elapsed=elapsed, + window=self._window_seconds, + ) + return GuardCheckOutcome( + passed=False, + mechanism=_MECHANISM, + message=( + f"Duplicate delegation " + f"({delegator_id!r} -> {delegatee_id!r}, " + f"{task_title!r}) within " + f"{self._window_seconds}s window" + ), + ) + return GuardCheckOutcome(passed=True, mechanism=_MECHANISM) + + def record( + self, + delegator_id: str, + delegatee_id: str, + task_title: str, + ) -> None: + """Record a delegation for future dedup checks. + + Args: + delegator_id: ID of the delegating agent. + delegatee_id: ID of the target agent. + task_title: Title of the task being delegated. + """ + key = (delegator_id, delegatee_id, task_title) + self._records[key] = self._clock() diff --git a/src/ai_company/communication/loop_prevention/depth.py b/src/ai_company/communication/loop_prevention/depth.py new file mode 100644 index 0000000000..c85c22611e --- /dev/null +++ b/src/ai_company/communication/loop_prevention/depth.py @@ -0,0 +1,30 @@ +"""Max delegation depth check (pure function).""" + +from ai_company.communication.loop_prevention.models import GuardCheckOutcome + +_MECHANISM = "max_depth" + + +def check_delegation_depth( + delegation_chain: tuple[str, ...], + max_depth: int, +) -> GuardCheckOutcome: + """Check whether the delegation chain exceeds maximum depth. + + Args: + delegation_chain: Current chain of delegator agent IDs. + max_depth: Maximum allowed chain length. + + Returns: + Outcome with passed=True if within limit. + """ + if len(delegation_chain) >= max_depth: + return GuardCheckOutcome( + passed=False, + mechanism=_MECHANISM, + message=( + f"Delegation chain length {len(delegation_chain)} " + f"reaches or exceeds max depth {max_depth}" + ), + ) + return GuardCheckOutcome(passed=True, mechanism=_MECHANISM) diff --git a/src/ai_company/communication/loop_prevention/guard.py b/src/ai_company/communication/loop_prevention/guard.py new file mode 100644 index 0000000000..3fea0934db --- /dev/null +++ b/src/ai_company/communication/loop_prevention/guard.py @@ -0,0 +1,113 @@ +"""Delegation guard orchestrating all loop prevention mechanisms.""" + +from ai_company.communication.config import LoopPreventionConfig # noqa: TC001 +from ai_company.communication.loop_prevention.ancestry import check_ancestry +from ai_company.communication.loop_prevention.circuit_breaker import ( + DelegationCircuitBreaker, +) +from ai_company.communication.loop_prevention.dedup import ( + DelegationDeduplicator, +) +from ai_company.communication.loop_prevention.depth import ( + check_delegation_depth, +) +from ai_company.communication.loop_prevention.models import GuardCheckOutcome +from ai_company.communication.loop_prevention.rate_limit import ( + DelegationRateLimiter, +) +from ai_company.observability import get_logger +from ai_company.observability.events.delegation import ( + DELEGATION_LOOP_BLOCKED, +) + +logger = get_logger(__name__) + +_SUCCESS_MECHANISM = "all_passed" + + +class DelegationGuard: + """Orchestrates all five loop prevention mechanisms. + + Checks run in order: ancestry, depth, dedup, rate_limit, + circuit_breaker. Returns the first failure or an overall success. + + Args: + config: Loop prevention configuration. + """ + + __slots__ = ( + "_circuit_breaker", + "_config", + "_deduplicator", + "_rate_limiter", + ) + + def __init__(self, config: LoopPreventionConfig) -> None: + self._config = config + self._deduplicator = DelegationDeduplicator( + window_seconds=config.dedup_window_seconds, + ) + self._rate_limiter = DelegationRateLimiter(config.rate_limit) + self._circuit_breaker = DelegationCircuitBreaker( + config.circuit_breaker, + ) + + def check( + self, + delegation_chain: tuple[str, ...], + delegator_id: str, + delegatee_id: str, + task_title: str, + ) -> GuardCheckOutcome: + """Run all loop prevention checks. + + Returns the first failing outcome, or a success outcome if + all checks pass. + + Args: + delegation_chain: Current delegation ancestry. + delegator_id: ID of the delegating agent. + delegatee_id: ID of the proposed delegatee. + task_title: Title of the task being delegated. + + Returns: + First failing outcome or an all-passed success. + """ + checks = [ + check_ancestry(delegation_chain, delegatee_id), + check_delegation_depth(delegation_chain, self._config.max_delegation_depth), + self._deduplicator.check(delegator_id, delegatee_id, task_title), + self._rate_limiter.check(delegator_id, delegatee_id), + self._circuit_breaker.check(delegator_id, delegatee_id), + ] + for outcome in checks: + if not outcome.passed: + logger.info( + DELEGATION_LOOP_BLOCKED, + mechanism=outcome.mechanism, + delegator=delegator_id, + delegatee=delegatee_id, + message=outcome.message, + ) + return outcome + return GuardCheckOutcome( + passed=True, + mechanism=_SUCCESS_MECHANISM, + ) + + def record_delegation( + self, + delegator_id: str, + delegatee_id: str, + task_title: str, + ) -> None: + """Record a successful delegation in all stateful mechanisms. + + Args: + delegator_id: ID of the delegating agent. + delegatee_id: ID of the target agent. + task_title: Title of the delegated task. + """ + self._deduplicator.record(delegator_id, delegatee_id, task_title) + self._rate_limiter.record(delegator_id, delegatee_id) + self._circuit_breaker.record_bounce(delegator_id, delegatee_id) diff --git a/src/ai_company/communication/loop_prevention/models.py b/src/ai_company/communication/loop_prevention/models.py new file mode 100644 index 0000000000..cdfd14fbfc --- /dev/null +++ b/src/ai_company/communication/loop_prevention/models.py @@ -0,0 +1,26 @@ +"""Loop prevention check outcome model.""" + +from pydantic import BaseModel, ConfigDict, Field + +from ai_company.core.types import NotBlankStr # noqa: TC001 + + +class GuardCheckOutcome(BaseModel): + """Result of a single loop prevention check. + + Attributes: + passed: Whether the check passed (delegation allowed). + mechanism: Name of the mechanism that produced this outcome. + message: Human-readable detail (empty on success). + """ + + model_config = ConfigDict(frozen=True) + + passed: bool = Field(description="Whether the check passed") + mechanism: NotBlankStr = Field( + description="Mechanism name (e.g. 'max_depth', 'ancestry')", + ) + message: str = Field( + default="", + description="Human-readable detail", + ) diff --git a/src/ai_company/communication/loop_prevention/rate_limit.py b/src/ai_company/communication/loop_prevention/rate_limit.py new file mode 100644 index 0000000000..8fb602bcc9 --- /dev/null +++ b/src/ai_company/communication/loop_prevention/rate_limit.py @@ -0,0 +1,104 @@ +"""Per-pair delegation rate limiter.""" + +import time +from collections.abc import Callable # noqa: TC003 + +from ai_company.communication.config import RateLimitConfig # noqa: TC001 +from ai_company.communication.loop_prevention.models import GuardCheckOutcome +from ai_company.observability import get_logger +from ai_company.observability.events.delegation import ( + DELEGATION_LOOP_RATE_LIMITED, +) + +logger = get_logger(__name__) + +_MECHANISM = "rate_limit" +_WINDOW_SECONDS = 60.0 + + +def _pair_key(a: str, b: str) -> tuple[str, str]: + """Create a canonical sorted key for an agent pair.""" + return (min(a, b), max(a, b)) + + +class DelegationRateLimiter: + """Per-pair rate limit with burst allowance. + + The key is the sorted (a, b) agent pair. Counts delegations within + the last 60-second window. If the count exceeds + ``max_per_pair_per_minute + burst_allowance``, the check fails. + + Args: + config: Rate limit configuration. + clock: Monotonic clock function for deterministic testing. + """ + + __slots__ = ("_clock", "_config", "_timestamps") + + def __init__( + self, + config: RateLimitConfig, + *, + clock: Callable[[], float] = time.monotonic, + ) -> None: + self._config = config + self._clock = clock + self._timestamps: dict[tuple[str, str], list[float]] = {} + + def check( + self, + delegator_id: str, + delegatee_id: str, + ) -> GuardCheckOutcome: + """Check whether the pair has exceeded the rate limit. + + Args: + delegator_id: ID of the delegating agent. + delegatee_id: ID of the target agent. + + Returns: + Outcome with passed=False if rate limit exceeded. + """ + key = _pair_key(delegator_id, delegatee_id) + now = self._clock() + cutoff = now - _WINDOW_SECONDS + timestamps = self._timestamps.get(key, []) + recent = [t for t in timestamps if t > cutoff] + limit = self._config.max_per_pair_per_minute + self._config.burst_allowance + if len(recent) >= limit: + logger.info( + DELEGATION_LOOP_RATE_LIMITED, + delegator=delegator_id, + delegatee=delegatee_id, + count=len(recent), + limit=limit, + ) + return GuardCheckOutcome( + passed=False, + mechanism=_MECHANISM, + message=( + f"Rate limit exceeded for pair " + f"({delegator_id!r}, {delegatee_id!r}): " + f"{len(recent)} delegations in last 60s " + f"(limit {limit})" + ), + ) + return GuardCheckOutcome(passed=True, mechanism=_MECHANISM) + + def record( + self, + delegator_id: str, + delegatee_id: str, + ) -> None: + """Record a delegation timestamp for the pair. + + Args: + delegator_id: ID of the delegating agent. + delegatee_id: ID of the target agent. + """ + key = _pair_key(delegator_id, delegatee_id) + now = self._clock() + cutoff = now - _WINDOW_SECONDS + timestamps = self._timestamps.get(key, []) + # Prune expired entries on write + self._timestamps[key] = [t for t in timestamps if t > cutoff] + [now] diff --git a/src/ai_company/core/task.py b/src/ai_company/core/task.py index bcbd1ca776..cdc06a4c30 100644 --- a/src/ai_company/core/task.py +++ b/src/ai_company/core/task.py @@ -122,6 +122,14 @@ class Task(BaseModel): default=TaskStatus.CREATED, description="Current lifecycle status", ) + parent_task_id: NotBlankStr | None = Field( + default=None, + description="Parent task ID when created via delegation", + ) + delegation_chain: tuple[NotBlankStr, ...] = Field( + default=(), + description="Ordered agent IDs of delegators (root first)", + ) @model_validator(mode="after") def _validate_deadline_format(self) -> Self: @@ -153,6 +161,25 @@ def _validate_collections(self) -> Self: raise ValueError(msg) return self + @model_validator(mode="after") + def _validate_delegation_fields(self) -> Self: + """Validate delegation-related field constraints.""" + if self.parent_task_id is not None and self.parent_task_id == self.id: + msg = f"Task {self.id!r} cannot be its own parent" + raise ValueError(msg) + if len(self.delegation_chain) != len(set(self.delegation_chain)): + dupes = sorted( + a for a, c in Counter(self.delegation_chain).items() if c > 1 + ) + msg = f"Duplicate entries in delegation_chain: {dupes}" + raise ValueError(msg) + if self.assigned_to is not None and self.assigned_to in self.delegation_chain: + msg = ( + f"assigned_to {self.assigned_to!r} must not appear in delegation_chain" + ) + raise ValueError(msg) + return self + @model_validator(mode="after") def _validate_assignment_consistency(self) -> Self: """Ensure assigned_to is consistent with status. diff --git a/src/ai_company/observability/events/delegation.py b/src/ai_company/observability/events/delegation.py new file mode 100644 index 0000000000..3963a253fb --- /dev/null +++ b/src/ai_company/observability/events/delegation.py @@ -0,0 +1,23 @@ +"""Delegation event constants.""" + +from typing import Final + +# Delegation lifecycle +DELEGATION_REQUESTED: Final[str] = "delegation.requested" +DELEGATION_AUTHORIZED: Final[str] = "delegation.authorized" +DELEGATION_AUTHORITY_DENIED: Final[str] = "delegation.authority_denied" +DELEGATION_CREATED: Final[str] = "delegation.created" +DELEGATION_RESULT_SENT: Final[str] = "delegation.result_sent" + +# Loop prevention +DELEGATION_LOOP_BLOCKED: Final[str] = "delegation.loop.blocked" +DELEGATION_LOOP_DEPTH_EXCEEDED: Final[str] = "delegation.loop.depth_exceeded" +DELEGATION_LOOP_ANCESTRY_BLOCKED: Final[str] = "delegation.loop.ancestry_blocked" +DELEGATION_LOOP_DEDUP_BLOCKED: Final[str] = "delegation.loop.dedup_blocked" +DELEGATION_LOOP_RATE_LIMITED: Final[str] = "delegation.loop.rate_limited" +DELEGATION_LOOP_CIRCUIT_OPEN: Final[str] = "delegation.loop.circuit_open" +DELEGATION_LOOP_ESCALATED: Final[str] = "delegation.loop.escalated" + +# Hierarchy +DELEGATION_HIERARCHY_BUILT: Final[str] = "delegation.hierarchy.built" +DELEGATION_HIERARCHY_CYCLE: Final[str] = "delegation.hierarchy.cycle" diff --git a/tests/integration/communication/__init__.py b/tests/integration/communication/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integration/communication/test_delegation_integration.py b/tests/integration/communication/test_delegation_integration.py new file mode 100644 index 0000000000..7a426ec935 --- /dev/null +++ b/tests/integration/communication/test_delegation_integration.py @@ -0,0 +1,373 @@ +"""Integration tests for hierarchical delegation with loop prevention.""" + +from datetime import date + +import pytest + +from ai_company.communication.config import ( + CircuitBreakerConfig, + HierarchyConfig, + LoopPreventionConfig, + RateLimitConfig, +) +from ai_company.communication.delegation.authority import ( + AuthorityValidator, +) +from ai_company.communication.delegation.hierarchy import ( + HierarchyResolver, +) +from ai_company.communication.delegation.models import ( + DelegationRequest, +) +from ai_company.communication.delegation.service import ( + DelegationService, +) +from ai_company.communication.loop_prevention.guard import ( + DelegationGuard, +) +from ai_company.core.agent import AgentIdentity, ModelConfig +from ai_company.core.company import ( + Company, + CompanyConfig, + Department, + Team, +) +from ai_company.core.enums import SeniorityLevel, TaskStatus, TaskType +from ai_company.core.role import Authority +from ai_company.core.task import Task + +pytestmark = pytest.mark.timeout(30) + + +def _model_config() -> ModelConfig: + return ModelConfig( + provider="test-provider", + model_id="test-small-001", + ) + + +def _make_agent( + name: str, + role: str, + *, + level: SeniorityLevel = SeniorityLevel.MID, + can_delegate_to: tuple[str, ...] = (), +) -> AgentIdentity: + return AgentIdentity( + name=name, + role=role, + department="Engineering", + level=level, + model=_model_config(), + hiring_date=date(2026, 1, 1), + authority=Authority(can_delegate_to=can_delegate_to), + ) + + +def _make_task(**overrides: object) -> Task: + defaults: dict[str, object] = { + "id": "task-root", + "title": "Build authentication system", + "description": "Implement full auth flow", + "type": TaskType.DEVELOPMENT, + "project": "proj-auth", + "created_by": "ceo", + } + defaults.update(overrides) + return Task(**defaults) # type: ignore[arg-type] + + +def _build_three_level_service() -> tuple[ + DelegationService, + HierarchyResolver, + dict[str, AgentIdentity], +]: + """Build a 3-level hierarchy: CEO → CTO → Dev. + + Returns: + service, hierarchy, agents dict. + """ + company = Company( + name="Test Corp", + departments=( + Department( + name="Engineering", + head="ceo", + budget_percent=100.0, + teams=( + Team( + name="platform", + lead="cto", + members=("dev",), + ), + ), + ), + ), + config=CompanyConfig(budget_monthly=100.0), + ) + hierarchy = HierarchyResolver(company) + hierarchy_config = HierarchyConfig( + enforce_chain_of_command=True, + allow_skip_level=True, + ) + authority_validator = AuthorityValidator(hierarchy, hierarchy_config) + guard = DelegationGuard( + LoopPreventionConfig( + max_delegation_depth=5, + rate_limit=RateLimitConfig(max_per_pair_per_minute=10, burst_allowance=3), + circuit_breaker=CircuitBreakerConfig( + bounce_threshold=3, cooldown_seconds=300 + ), + ), + ) + service = DelegationService( + hierarchy=hierarchy, + authority_validator=authority_validator, + guard=guard, + ) + agents = { + "ceo": _make_agent("ceo", "CEO", level=SeniorityLevel.VP), + "cto": _make_agent("cto", "CTO", level=SeniorityLevel.DIRECTOR), + "dev": _make_agent("dev", "Developer", level=SeniorityLevel.MID), + } + return service, hierarchy, agents + + +@pytest.mark.integration +class TestFullDelegationFlow: + """End-to-end delegation through a 3-level hierarchy.""" + + def test_ceo_to_cto_to_dev(self) -> None: + """CEO delegates to CTO, CTO delegates to Dev.""" + service, _, agents = _build_three_level_service() + task = _make_task() + + # CEO → CTO + req1 = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + refinement="Focus on backend API", + ) + r1 = service.delegate(req1, agents["ceo"], agents["cto"]) + assert r1.success is True + sub1 = r1.delegated_task + assert sub1 is not None + assert sub1.parent_task_id == "task-root" + assert sub1.delegation_chain == ("ceo",) + assert sub1.status is TaskStatus.CREATED + assert "Focus on backend API" in sub1.description + + # CTO → Dev (different title to avoid dedup) + sub1_retitled = Task( + id=sub1.id, + title="Build auth API endpoints", + description=sub1.description, + type=sub1.type, + project=sub1.project, + created_by=sub1.created_by, + parent_task_id=sub1.parent_task_id, + delegation_chain=sub1.delegation_chain, + ) + req2 = DelegationRequest( + delegator_id="cto", + delegatee_id="dev", + task=sub1_retitled, + ) + r2 = service.delegate(req2, agents["cto"], agents["dev"]) + assert r2.success is True + sub2 = r2.delegated_task + assert sub2 is not None + assert sub2.delegation_chain == ("ceo", "cto") + assert sub2.parent_task_id == sub1.id + + # Verify audit trail + trail = service.get_audit_trail() + assert len(trail) == 2 + + def test_ancestry_prevents_back_delegation(self) -> None: + """Dev cannot delegate back to CEO (ancestry block).""" + service, _, agents = _build_three_level_service() + task = _make_task() + + # CEO → CTO + req1 = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + r1 = service.delegate(req1, agents["ceo"], agents["cto"]) + sub1 = r1.delegated_task + assert sub1 is not None + + # CTO → Dev + sub1_new = Task( + id=sub1.id, + title="Auth: implement login", + description=sub1.description, + type=sub1.type, + project=sub1.project, + created_by=sub1.created_by, + parent_task_id=sub1.parent_task_id, + delegation_chain=sub1.delegation_chain, + ) + req2 = DelegationRequest( + delegator_id="cto", + delegatee_id="dev", + task=sub1_new, + ) + r2 = service.delegate(req2, agents["cto"], agents["dev"]) + sub2 = r2.delegated_task + assert sub2 is not None + # chain is now ("ceo", "cto") + + # Dev → CEO: should be blocked by ancestry + # (ceo is in delegation_chain) + req3 = DelegationRequest( + delegator_id="dev", + delegatee_id="ceo", + task=sub2, + ) + r3 = service.delegate(req3, agents["dev"], agents["ceo"]) + assert r3.success is False + # Blocked by either ancestry or authority + assert r3.blocked_by is not None + + def test_dedup_prevents_repeated_delegation(self) -> None: + """Same delegation request is rejected on second attempt.""" + service, _, agents = _build_three_level_service() + task = _make_task() + req = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + r1 = service.delegate(req, agents["ceo"], agents["cto"]) + assert r1.success is True + + r2 = service.delegate(req, agents["ceo"], agents["cto"]) + assert r2.success is False + assert r2.blocked_by == "dedup" + + +@pytest.mark.integration +class TestCircuitBreakerIntegration: + """Circuit breaker triggers after repeated bounces.""" + + def test_circuit_opens_after_threshold(self) -> None: + company = Company( + name="Test Corp", + departments=( + Department( + name="Engineering", + head="ceo", + budget_percent=100.0, + teams=( + Team( + name="platform", + lead="cto", + members=("dev",), + ), + ), + ), + ), + config=CompanyConfig(budget_monthly=100.0), + ) + hierarchy = HierarchyResolver(company) + hierarchy_config = HierarchyConfig( + enforce_chain_of_command=True, + allow_skip_level=False, + ) + authority_validator = AuthorityValidator(hierarchy, hierarchy_config) + guard = DelegationGuard( + LoopPreventionConfig( + max_delegation_depth=10, + rate_limit=RateLimitConfig( + max_per_pair_per_minute=100, + burst_allowance=100, + ), + circuit_breaker=CircuitBreakerConfig( + bounce_threshold=3, cooldown_seconds=300 + ), + ), + ) + service = DelegationService( + hierarchy=hierarchy, + authority_validator=authority_validator, + guard=guard, + ) + ceo = _make_agent("ceo", "CEO", level=SeniorityLevel.VP) + cto = _make_agent("cto", "CTO", level=SeniorityLevel.DIRECTOR) + + # Perform 3 delegations (= bounce_threshold) + for i in range(3): + task = _make_task( + id=f"task-{i}", + title=f"Task variant {i}", + ) + req = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + result = service.delegate(req, ceo, cto) + assert result.success is True + + # 4th delegation: circuit breaker should block + task4 = _make_task(id="task-4", title="Task after circuit") + req4 = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task4, + ) + r4 = service.delegate(req4, ceo, cto) + assert r4.success is False + assert r4.blocked_by == "circuit_breaker" + + +@pytest.mark.integration +class TestDelegationChainValidation: + """Validate delegation chain grows correctly across hops.""" + + def test_chain_carries_full_ancestry(self) -> None: + service, _, agents = _build_three_level_service() + + # Start with root task + root = _make_task() + assert root.delegation_chain == () + + # CEO → CTO + req1 = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=root, + ) + r1 = service.delegate(req1, agents["ceo"], agents["cto"]) + sub1 = r1.delegated_task + assert sub1 is not None + assert sub1.delegation_chain == ("ceo",) + + # CTO → Dev + sub1_retitled = Task( + id=sub1.id, + title="Auth subtask for dev", + description=sub1.description, + type=sub1.type, + project=sub1.project, + created_by=sub1.created_by, + parent_task_id=sub1.parent_task_id, + delegation_chain=sub1.delegation_chain, + ) + req2 = DelegationRequest( + delegator_id="cto", + delegatee_id="dev", + task=sub1_retitled, + ) + r2 = service.delegate(req2, agents["cto"], agents["dev"]) + sub2 = r2.delegated_task + assert sub2 is not None + assert sub2.delegation_chain == ("ceo", "cto") + + # Verify parent chain + assert sub2.parent_task_id == sub1.id + assert sub1.parent_task_id == "task-root" diff --git a/tests/unit/communication/delegation/__init__.py b/tests/unit/communication/delegation/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/communication/delegation/test_authority.py b/tests/unit/communication/delegation/test_authority.py new file mode 100644 index 0000000000..630caca9ca --- /dev/null +++ b/tests/unit/communication/delegation/test_authority.py @@ -0,0 +1,180 @@ +"""Tests for AuthorityValidator.""" + +from datetime import date + +import pytest + +from ai_company.communication.config import HierarchyConfig +from ai_company.communication.delegation.authority import ( + AuthorityValidator, +) +from ai_company.communication.delegation.hierarchy import ( + HierarchyResolver, +) +from ai_company.core.agent import AgentIdentity, ModelConfig +from ai_company.core.company import ( + Company, + CompanyConfig, + Department, + Team, +) +from ai_company.core.enums import SeniorityLevel +from ai_company.core.role import Authority + +pytestmark = pytest.mark.timeout(30) + + +def _make_agent( + name: str, + role: str = "developer", + *, + can_delegate_to: tuple[str, ...] = (), +) -> AgentIdentity: + return AgentIdentity( + name=name, + role=role, + department="Engineering", + level=SeniorityLevel.MID, + model=ModelConfig( + provider="test-provider", + model_id="test-small-001", + ), + hiring_date=date(2026, 1, 1), + authority=Authority(can_delegate_to=can_delegate_to), + ) + + +def _build_resolver() -> HierarchyResolver: + """Build hierarchy: cto -> backend_lead -> dev.""" + company = Company( + name="Test Corp", + departments=( + Department( + name="Engineering", + head="cto", + budget_percent=60.0, + teams=( + Team( + name="backend", + lead="backend_lead", + members=("dev", "jr_dev"), + ), + ), + ), + ), + config=CompanyConfig(budget_monthly=100.0), + ) + return HierarchyResolver(company) + + +@pytest.mark.unit +class TestAuthorityValidatorHierarchy: + def test_direct_report_allowed(self) -> None: + resolver = _build_resolver() + config = HierarchyConfig( + enforce_chain_of_command=True, + allow_skip_level=False, + ) + validator = AuthorityValidator(resolver, config) + delegator = _make_agent("backend_lead", "lead") + delegatee = _make_agent("dev") + result = validator.validate(delegator, delegatee) + assert result.allowed is True + + def test_non_report_denied(self) -> None: + resolver = _build_resolver() + config = HierarchyConfig( + enforce_chain_of_command=True, + allow_skip_level=False, + ) + validator = AuthorityValidator(resolver, config) + delegator = _make_agent("dev") + delegatee = _make_agent("cto") + result = validator.validate(delegator, delegatee) + assert result.allowed is False + assert "direct report" in result.reason + + def test_skip_level_denied_without_config(self) -> None: + resolver = _build_resolver() + config = HierarchyConfig( + enforce_chain_of_command=True, + allow_skip_level=False, + ) + validator = AuthorityValidator(resolver, config) + delegator = _make_agent("cto") + delegatee = _make_agent("dev") + result = validator.validate(delegator, delegatee) + assert result.allowed is False + + def test_skip_level_allowed_with_config(self) -> None: + resolver = _build_resolver() + config = HierarchyConfig( + enforce_chain_of_command=True, + allow_skip_level=True, + ) + validator = AuthorityValidator(resolver, config) + delegator = _make_agent("cto") + delegatee = _make_agent("dev") + result = validator.validate(delegator, delegatee) + assert result.allowed is True + + def test_chain_of_command_disabled(self) -> None: + """When chain of command is not enforced, any pair is allowed.""" + resolver = _build_resolver() + config = HierarchyConfig( + enforce_chain_of_command=False, + allow_skip_level=False, + ) + validator = AuthorityValidator(resolver, config) + delegator = _make_agent("dev") + delegatee = _make_agent("cto") + result = validator.validate(delegator, delegatee) + assert result.allowed is True + + +@pytest.mark.unit +class TestAuthorityValidatorRoles: + def test_role_in_can_delegate_to_allowed(self) -> None: + resolver = _build_resolver() + config = HierarchyConfig( + enforce_chain_of_command=True, + allow_skip_level=False, + ) + validator = AuthorityValidator(resolver, config) + delegator = _make_agent( + "backend_lead", + "lead", + can_delegate_to=("developer",), + ) + delegatee = _make_agent("dev", "developer") + result = validator.validate(delegator, delegatee) + assert result.allowed is True + + def test_role_not_in_can_delegate_to_denied(self) -> None: + resolver = _build_resolver() + config = HierarchyConfig( + enforce_chain_of_command=True, + allow_skip_level=False, + ) + validator = AuthorityValidator(resolver, config) + delegator = _make_agent( + "backend_lead", + "lead", + can_delegate_to=("qa",), + ) + delegatee = _make_agent("dev", "developer") + result = validator.validate(delegator, delegatee) + assert result.allowed is False + assert "can_delegate_to" in result.reason + + def test_empty_can_delegate_to_allows_all_roles(self) -> None: + resolver = _build_resolver() + config = HierarchyConfig( + enforce_chain_of_command=True, + allow_skip_level=False, + ) + validator = AuthorityValidator(resolver, config) + delegator = _make_agent("backend_lead", "lead") + delegatee = _make_agent("dev", "any_role") + result = validator.validate(delegator, delegatee) + assert result.allowed is True diff --git a/tests/unit/communication/delegation/test_hierarchy.py b/tests/unit/communication/delegation/test_hierarchy.py new file mode 100644 index 0000000000..c7f7c749a2 --- /dev/null +++ b/tests/unit/communication/delegation/test_hierarchy.py @@ -0,0 +1,267 @@ +"""Tests for HierarchyResolver.""" + +import pytest + +from ai_company.communication.delegation.hierarchy import ( + HierarchyResolver, +) +from ai_company.communication.errors import HierarchyResolutionError +from ai_company.core.company import ( + Company, + CompanyConfig, + Department, + ReportingLine, + Team, +) + +pytestmark = pytest.mark.timeout(30) + + +def _make_company( + departments: tuple[Department, ...] = (), +) -> Company: + """Create a minimal Company with given departments.""" + return Company( + name="Test Corp", + departments=departments, + config=CompanyConfig(budget_monthly=100.0), + ) + + +def _eng_department() -> Department: + """Engineering department with 2 teams and reporting lines.""" + return Department( + name="Engineering", + head="cto", + budget_percent=60.0, + teams=( + Team( + name="backend", + lead="backend_lead", + members=("sr_dev", "jr_dev"), + ), + Team( + name="frontend", + lead="frontend_lead", + members=("ui_dev",), + ), + ), + ) + + +def _qa_department() -> Department: + """QA department with one team.""" + return Department( + name="QA", + head="qa_head", + budget_percent=20.0, + teams=( + Team( + name="testing", + lead="qa_lead", + members=("qa_eng",), + ), + ), + ) + + +@pytest.mark.unit +class TestHierarchyResolverConstruction: + def test_builds_from_single_department(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("backend_lead") == "cto" + + def test_builds_from_multi_department(self) -> None: + company = _make_company(departments=(_eng_department(), _qa_department())) + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("qa_lead") == "qa_head" + + def test_empty_company(self) -> None: + company = _make_company() + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("anyone") is None + + +@pytest.mark.unit +class TestGetSupervisor: + def test_team_member_supervisor_is_lead(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("sr_dev") == "backend_lead" + assert resolver.get_supervisor("jr_dev") == "backend_lead" + + def test_team_lead_supervisor_is_dept_head(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("backend_lead") == "cto" + assert resolver.get_supervisor("frontend_lead") == "cto" + + def test_dept_head_has_no_supervisor(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("cto") is None + + def test_unknown_agent_returns_none(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("nonexistent") is None + + +@pytest.mark.unit +class TestGetDirectReports: + def test_dept_head_reports(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + reports = resolver.get_direct_reports("cto") + assert "backend_lead" in reports + assert "frontend_lead" in reports + + def test_team_lead_reports(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + reports = resolver.get_direct_reports("backend_lead") + assert "sr_dev" in reports + assert "jr_dev" in reports + + def test_leaf_agent_no_reports(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_direct_reports("jr_dev") == () + + def test_unknown_agent_empty(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_direct_reports("nonexistent") == () + + +@pytest.mark.unit +class TestIsDirectReport: + def test_direct_report_true(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.is_direct_report("backend_lead", "sr_dev") + + def test_not_direct_report_false(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert not resolver.is_direct_report("cto", "sr_dev") + + +@pytest.mark.unit +class TestIsSubordinate: + def test_direct_subordinate(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.is_subordinate("backend_lead", "sr_dev") + + def test_skip_level_subordinate(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.is_subordinate("cto", "jr_dev") + + def test_not_subordinate(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert not resolver.is_subordinate("sr_dev", "cto") + + def test_same_agent_not_subordinate(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert not resolver.is_subordinate("cto", "cto") + + def test_cross_department_not_subordinate(self) -> None: + company = _make_company(departments=(_eng_department(), _qa_department())) + resolver = HierarchyResolver(company) + assert not resolver.is_subordinate("cto", "qa_eng") + + +@pytest.mark.unit +class TestGetAncestors: + def test_leaf_agent_ancestors(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + ancestors = resolver.get_ancestors("jr_dev") + assert ancestors == ("backend_lead", "cto") + + def test_team_lead_ancestors(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + ancestors = resolver.get_ancestors("backend_lead") + assert ancestors == ("cto",) + + def test_dept_head_no_ancestors(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_ancestors("cto") == () + + def test_unknown_agent_no_ancestors(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_ancestors("nonexistent") == () + + +@pytest.mark.unit +class TestGetDelegationDepth: + def test_direct_depth_one(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_delegation_depth("backend_lead", "sr_dev") == 1 + + def test_skip_level_depth_two(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_delegation_depth("cto", "jr_dev") == 2 + + def test_not_below_returns_none(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_delegation_depth("jr_dev", "cto") is None + + def test_same_agent_returns_none(self) -> None: + company = _make_company(departments=(_eng_department(),)) + resolver = HierarchyResolver(company) + assert resolver.get_delegation_depth("cto", "cto") is None + + +@pytest.mark.unit +class TestExplicitReportingLines: + def test_reporting_line_overrides_team_structure(self) -> None: + """Explicit reporting line takes precedence over team lead.""" + dept = Department( + name="Engineering", + head="cto", + budget_percent=50.0, + teams=( + Team( + name="backend", + lead="backend_lead", + members=("sr_dev",), + ), + ), + reporting_lines=( + ReportingLine( + subordinate="sr_dev", + supervisor="cto", + ), + ), + ) + company = _make_company(departments=(dept,)) + resolver = HierarchyResolver(company) + # sr_dev should report to cto, not backend_lead + assert resolver.get_supervisor("sr_dev") == "cto" + + +@pytest.mark.unit +class TestCycleDetection: + def test_cycle_raises_hierarchy_error(self) -> None: + dept = Department( + name="Engineering", + head="a", + budget_percent=50.0, + teams=(Team(name="t1", lead="b", members=("c",)),), + reporting_lines=(ReportingLine(subordinate="a", supervisor="c"),), + ) + company = _make_company(departments=(dept,)) + with pytest.raises(HierarchyResolutionError, match="Cycle"): + HierarchyResolver(company) diff --git a/tests/unit/communication/delegation/test_models.py b/tests/unit/communication/delegation/test_models.py new file mode 100644 index 0000000000..9219031ed5 --- /dev/null +++ b/tests/unit/communication/delegation/test_models.py @@ -0,0 +1,189 @@ +"""Tests for delegation models.""" + +from datetime import UTC, datetime + +import pytest +from pydantic import ValidationError + +from ai_company.communication.delegation.models import ( + DelegationRecord, + DelegationRequest, + DelegationResult, +) +from ai_company.core.enums import TaskType +from ai_company.core.task import Task + +pytestmark = pytest.mark.timeout(30) + + +def _make_task(**overrides: object) -> Task: + defaults: dict[str, object] = { + "id": "task-1", + "title": "Test task", + "description": "A test task", + "type": TaskType.DEVELOPMENT, + "project": "proj-1", + "created_by": "pm-1", + } + defaults.update(overrides) + return Task(**defaults) # type: ignore[arg-type] + + +@pytest.mark.unit +class TestDelegationRequest: + def test_minimal_valid(self) -> None: + task = _make_task() + req = DelegationRequest( + delegator_id="cto", + delegatee_id="dev", + task=task, + ) + assert req.delegator_id == "cto" + assert req.delegatee_id == "dev" + assert req.task is task + assert req.refinement == "" + assert req.constraints == () + + def test_with_refinement_and_constraints(self) -> None: + task = _make_task() + req = DelegationRequest( + delegator_id="cto", + delegatee_id="dev", + task=task, + refinement="Focus on performance", + constraints=("no-external-deps", "max-2-files"), + ) + assert req.refinement == "Focus on performance" + assert len(req.constraints) == 2 + + def test_frozen(self) -> None: + task = _make_task() + req = DelegationRequest( + delegator_id="cto", + delegatee_id="dev", + task=task, + ) + with pytest.raises(ValidationError): + req.delegator_id = "new" # type: ignore[misc] + + def test_blank_delegator_rejected(self) -> None: + task = _make_task() + with pytest.raises(ValidationError): + DelegationRequest( + delegator_id=" ", + delegatee_id="dev", + task=task, + ) + + def test_blank_delegatee_rejected(self) -> None: + task = _make_task() + with pytest.raises(ValidationError): + DelegationRequest( + delegator_id="cto", + delegatee_id="", + task=task, + ) + + +@pytest.mark.unit +class TestDelegationResult: + def test_success_result(self) -> None: + task = _make_task() + result = DelegationResult( + success=True, + delegated_task=task, + ) + assert result.success is True + assert result.delegated_task is task + assert result.rejection_reason is None + assert result.blocked_by is None + + def test_failure_result(self) -> None: + result = DelegationResult( + success=False, + rejection_reason="Authority denied", + blocked_by="hierarchy", + ) + assert result.success is False + assert result.delegated_task is None + assert result.rejection_reason == "Authority denied" + assert result.blocked_by == "hierarchy" + + def test_frozen(self) -> None: + result = DelegationResult(success=True) + with pytest.raises(ValidationError): + result.success = False # type: ignore[misc] + + +@pytest.mark.unit +class TestDelegationRecord: + def test_valid_record(self) -> None: + now = datetime.now(UTC) + record = DelegationRecord( + delegation_id="del-1", + delegator_id="cto", + delegatee_id="dev", + original_task_id="task-1", + delegated_task_id="task-2", + timestamp=now, + ) + assert record.delegation_id == "del-1" + assert record.delegator_id == "cto" + assert record.delegatee_id == "dev" + assert record.original_task_id == "task-1" + assert record.delegated_task_id == "task-2" + assert record.timestamp == now + assert record.refinement == "" + + def test_with_refinement(self) -> None: + now = datetime.now(UTC) + record = DelegationRecord( + delegation_id="del-2", + delegator_id="cto", + delegatee_id="dev", + original_task_id="task-1", + delegated_task_id="task-3", + timestamp=now, + refinement="Focus on API layer", + ) + assert record.refinement == "Focus on API layer" + + def test_frozen(self) -> None: + now = datetime.now(UTC) + record = DelegationRecord( + delegation_id="del-1", + delegator_id="cto", + delegatee_id="dev", + original_task_id="task-1", + delegated_task_id="task-2", + timestamp=now, + ) + with pytest.raises(ValidationError): + record.delegator_id = "new" # type: ignore[misc] + + def test_blank_ids_rejected(self) -> None: + now = datetime.now(UTC) + with pytest.raises(ValidationError): + DelegationRecord( + delegation_id=" ", + delegator_id="cto", + delegatee_id="dev", + original_task_id="task-1", + delegated_task_id="task-2", + timestamp=now, + ) + + def test_json_roundtrip(self) -> None: + now = datetime.now(UTC) + record = DelegationRecord( + delegation_id="del-1", + delegator_id="cto", + delegatee_id="dev", + original_task_id="task-1", + delegated_task_id="task-2", + timestamp=now, + ) + json_str = record.model_dump_json() + restored = DelegationRecord.model_validate_json(json_str) + assert restored.delegation_id == record.delegation_id + assert restored.timestamp == record.timestamp diff --git a/tests/unit/communication/delegation/test_service.py b/tests/unit/communication/delegation/test_service.py new file mode 100644 index 0000000000..f763de20c5 --- /dev/null +++ b/tests/unit/communication/delegation/test_service.py @@ -0,0 +1,396 @@ +"""Tests for DelegationService.""" + +from datetime import date + +import pytest + +from ai_company.communication.config import ( + CircuitBreakerConfig, + HierarchyConfig, + LoopPreventionConfig, + RateLimitConfig, +) +from ai_company.communication.delegation.authority import ( + AuthorityValidator, +) +from ai_company.communication.delegation.hierarchy import ( + HierarchyResolver, +) +from ai_company.communication.delegation.models import ( + DelegationRequest, +) +from ai_company.communication.delegation.service import ( + DelegationService, +) +from ai_company.communication.loop_prevention.guard import ( + DelegationGuard, +) +from ai_company.core.agent import AgentIdentity, ModelConfig +from ai_company.core.company import ( + Company, + CompanyConfig, + Department, + Team, +) +from ai_company.core.enums import SeniorityLevel, TaskStatus, TaskType +from ai_company.core.role import Authority +from ai_company.core.task import Task + +pytestmark = pytest.mark.timeout(30) + + +def _model_config() -> ModelConfig: + return ModelConfig( + provider="test-provider", + model_id="test-small-001", + ) + + +def _make_agent( + name: str, + role: str = "developer", + *, + can_delegate_to: tuple[str, ...] = (), +) -> AgentIdentity: + return AgentIdentity( + name=name, + role=role, + department="Engineering", + level=SeniorityLevel.MID, + model=_model_config(), + hiring_date=date(2026, 1, 1), + authority=Authority(can_delegate_to=can_delegate_to), + ) + + +def _make_task(**overrides: object) -> Task: + defaults: dict[str, object] = { + "id": "task-1", + "title": "Build feature X", + "description": "Implement the feature", + "type": TaskType.DEVELOPMENT, + "project": "proj-1", + "created_by": "ceo", + } + defaults.update(overrides) + return Task(**defaults) # type: ignore[arg-type] + + +def _build_service( + *, + enforce_chain: bool = True, + allow_skip: bool = False, + max_depth: int = 5, +) -> tuple[ + DelegationService, + HierarchyResolver, +]: + """Build a DelegationService with a 3-level hierarchy.""" + company = Company( + name="Test Corp", + departments=( + Department( + name="Engineering", + head="ceo", + budget_percent=100.0, + teams=( + Team( + name="platform", + lead="cto", + members=("dev",), + ), + ), + ), + ), + config=CompanyConfig(budget_monthly=100.0), + ) + hierarchy = HierarchyResolver(company) + hierarchy_config = HierarchyConfig( + enforce_chain_of_command=enforce_chain, + allow_skip_level=allow_skip, + ) + authority_validator = AuthorityValidator(hierarchy, hierarchy_config) + guard = DelegationGuard( + LoopPreventionConfig( + max_delegation_depth=max_depth, + rate_limit=RateLimitConfig(max_per_pair_per_minute=10, burst_allowance=3), + circuit_breaker=CircuitBreakerConfig( + bounce_threshold=3, cooldown_seconds=300 + ), + ), + ) + service = DelegationService( + hierarchy=hierarchy, + authority_validator=authority_validator, + guard=guard, + ) + return service, hierarchy + + +@pytest.mark.unit +class TestDelegationServiceSuccess: + def test_successful_delegation(self) -> None: + service, _ = _build_service() + task = _make_task() + delegator = _make_agent("ceo", "ceo") + delegatee = _make_agent("cto", "cto") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + result = service.delegate(request, delegator, delegatee) + assert result.success is True + assert result.delegated_task is not None + assert result.delegated_task.parent_task_id == "task-1" + assert result.delegated_task.delegation_chain == ("ceo",) + assert result.delegated_task.status is TaskStatus.CREATED + + def test_sub_task_inherits_properties(self) -> None: + service, _ = _build_service() + task = _make_task(budget_limit=50.0, deadline="2026-12-31") + delegator = _make_agent("ceo", "ceo") + delegatee = _make_agent("cto", "cto") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + result = service.delegate(request, delegator, delegatee) + sub = result.delegated_task + assert sub is not None + assert sub.budget_limit == 50.0 + assert sub.deadline == "2026-12-31" + assert sub.type is TaskType.DEVELOPMENT + + def test_refinement_appended_to_description(self) -> None: + service, _ = _build_service() + task = _make_task() + delegator = _make_agent("ceo", "ceo") + delegatee = _make_agent("cto", "cto") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + refinement="Focus on API layer", + ) + result = service.delegate(request, delegator, delegatee) + sub = result.delegated_task + assert sub is not None + assert "Focus on API layer" in sub.description + + def test_audit_trail_recorded(self) -> None: + service, _ = _build_service() + task = _make_task() + delegator = _make_agent("ceo", "ceo") + delegatee = _make_agent("cto", "cto") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + service.delegate(request, delegator, delegatee) + trail = service.get_audit_trail() + assert len(trail) == 1 + assert trail[0].delegator_id == "ceo" + assert trail[0].delegatee_id == "cto" + assert trail[0].original_task_id == "task-1" + + +@pytest.mark.unit +class TestDelegationServiceAuthority: + def test_authority_denied(self) -> None: + service, _ = _build_service() + task = _make_task() + # dev trying to delegate to ceo (not a report) + delegator = _make_agent("dev") + delegatee = _make_agent("ceo", "ceo") + request = DelegationRequest( + delegator_id="dev", + delegatee_id="ceo", + task=task, + ) + result = service.delegate(request, delegator, delegatee) + assert result.success is False + assert result.blocked_by == "authority" + + def test_role_permission_denied(self) -> None: + service, _ = _build_service() + task = _make_task() + delegator = _make_agent("ceo", "ceo", can_delegate_to=("qa",)) + delegatee = _make_agent("cto", "cto") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + result = service.delegate(request, delegator, delegatee) + assert result.success is False + assert result.blocked_by == "authority" + + +@pytest.mark.unit +class TestDelegationServiceLoopPrevention: + def test_ancestry_blocked(self) -> None: + service, _ = _build_service(allow_skip=True) + # First delegation: ceo -> cto + task1 = _make_task() + ceo = _make_agent("ceo", "ceo") + cto = _make_agent("cto", "cto") + req1 = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task1, + ) + r1 = service.delegate(req1, ceo, cto) + assert r1.success is True + + # Second: cto -> dev using sub-task + sub = r1.delegated_task + assert sub is not None + dev = _make_agent("dev") + req2 = DelegationRequest( + delegator_id="cto", + delegatee_id="dev", + task=sub, + ) + r2 = service.delegate(req2, cto, dev) + assert r2.success is True + + # Third: dev tries to delegate back to ceo → blocked + sub2 = r2.delegated_task + assert sub2 is not None + req3 = DelegationRequest( + delegator_id="dev", + delegatee_id="ceo", + task=sub2, + ) + # Chain of command won't matter here — ancestry blocks first + r3 = service.delegate(req3, dev, ceo) + # Either authority or ancestry blocks it + assert r3.success is False + + def test_depth_exceeded(self) -> None: + service, _ = _build_service(max_depth=1) + task = _make_task(delegation_chain=("root",)) + delegator = _make_agent("ceo", "ceo") + delegatee = _make_agent("cto", "cto") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + result = service.delegate(request, delegator, delegatee) + assert result.success is False + assert result.blocked_by == "max_depth" + + def test_dedup_blocked(self) -> None: + service, _ = _build_service() + task = _make_task() + delegator = _make_agent("ceo", "ceo") + delegatee = _make_agent("cto", "cto") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + # First succeeds + r1 = service.delegate(request, delegator, delegatee) + assert r1.success is True + # Second is dedup blocked (same delegator, delegatee, title) + r2 = service.delegate(request, delegator, delegatee) + assert r2.success is False + assert r2.blocked_by == "dedup" + + +@pytest.mark.unit +class TestDelegationServiceMultiHop: + def test_multi_level_delegation_chain(self) -> None: + """CEO → CTO → Dev: delegation chain grows correctly.""" + service, _ = _build_service(allow_skip=True) + task = _make_task() + ceo = _make_agent("ceo", "ceo") + cto = _make_agent("cto", "cto") + dev = _make_agent("dev") + + # CEO → CTO + req1 = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + r1 = service.delegate(req1, ceo, cto) + assert r1.success is True + sub1 = r1.delegated_task + assert sub1 is not None + assert sub1.delegation_chain == ("ceo",) + + # CTO → Dev (using sub-task, different title to avoid dedup) + sub1_new_title = Task( + id=sub1.id, + title="Build feature X - backend", + description=sub1.description, + type=sub1.type, + project=sub1.project, + created_by=sub1.created_by, + parent_task_id=sub1.parent_task_id, + delegation_chain=sub1.delegation_chain, + ) + req2 = DelegationRequest( + delegator_id="cto", + delegatee_id="dev", + task=sub1_new_title, + ) + r2 = service.delegate(req2, cto, dev) + assert r2.success is True + sub2 = r2.delegated_task + assert sub2 is not None + assert sub2.delegation_chain == ("ceo", "cto") + assert sub2.parent_task_id == sub1.id + + def test_audit_trail_multi_hop(self) -> None: + service, _ = _build_service(allow_skip=True) + task = _make_task() + ceo = _make_agent("ceo", "ceo") + cto = _make_agent("cto", "cto") + dev = _make_agent("dev") + + req1 = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + r1 = service.delegate(req1, ceo, cto) + sub1 = r1.delegated_task + assert sub1 is not None + + sub1_retitled = Task( + id=sub1.id, + title="Feature X - backend piece", + description=sub1.description, + type=sub1.type, + project=sub1.project, + created_by=sub1.created_by, + parent_task_id=sub1.parent_task_id, + delegation_chain=sub1.delegation_chain, + ) + req2 = DelegationRequest( + delegator_id="cto", + delegatee_id="dev", + task=sub1_retitled, + ) + service.delegate(req2, cto, dev) + + trail = service.get_audit_trail() + assert len(trail) == 2 + assert trail[0].delegator_id == "ceo" + assert trail[1].delegator_id == "cto" + + +@pytest.mark.unit +class TestDelegationServiceHelpers: + def test_get_supervisor_of(self) -> None: + service, _ = _build_service() + assert service.get_supervisor_of("cto") == "ceo" + assert service.get_supervisor_of("dev") == "cto" + assert service.get_supervisor_of("ceo") is None diff --git a/tests/unit/communication/loop_prevention/__init__.py b/tests/unit/communication/loop_prevention/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/communication/loop_prevention/test_ancestry.py b/tests/unit/communication/loop_prevention/test_ancestry.py new file mode 100644 index 0000000000..23277b9f20 --- /dev/null +++ b/tests/unit/communication/loop_prevention/test_ancestry.py @@ -0,0 +1,43 @@ +"""Tests for ancestry cycle detection check.""" + +import pytest + +from ai_company.communication.loop_prevention.ancestry import ( + check_ancestry, +) + +pytestmark = pytest.mark.timeout(30) + + +@pytest.mark.unit +class TestCheckAncestry: + def test_empty_chain_passes(self) -> None: + result = check_ancestry((), "agent-a") + assert result.passed is True + assert result.mechanism == "ancestry" + + def test_delegatee_not_in_chain_passes(self) -> None: + result = check_ancestry(("a", "b", "c"), "d") + assert result.passed is True + + def test_delegatee_in_chain_fails(self) -> None: + result = check_ancestry(("a", "b", "c"), "b") + assert result.passed is False + assert result.mechanism == "ancestry" + assert "'b'" in result.message + + def test_delegatee_is_root_fails(self) -> None: + result = check_ancestry(("root", "mid"), "root") + assert result.passed is False + + def test_delegatee_is_last_in_chain_fails(self) -> None: + result = check_ancestry(("a", "b"), "b") + assert result.passed is False + + def test_single_element_chain_match_fails(self) -> None: + result = check_ancestry(("x",), "x") + assert result.passed is False + + def test_single_element_chain_no_match_passes(self) -> None: + result = check_ancestry(("x",), "y") + assert result.passed is True diff --git a/tests/unit/communication/loop_prevention/test_circuit_breaker.py b/tests/unit/communication/loop_prevention/test_circuit_breaker.py new file mode 100644 index 0000000000..2e976bcc95 --- /dev/null +++ b/tests/unit/communication/loop_prevention/test_circuit_breaker.py @@ -0,0 +1,97 @@ +"""Tests for delegation circuit breaker.""" + +import pytest + +from ai_company.communication.config import CircuitBreakerConfig +from ai_company.communication.loop_prevention.circuit_breaker import ( + CircuitBreakerState, + DelegationCircuitBreaker, +) + +pytestmark = pytest.mark.timeout(30) + + +@pytest.mark.unit +class TestDelegationCircuitBreaker: + def test_initial_state_closed(self) -> None: + config = CircuitBreakerConfig(bounce_threshold=3, cooldown_seconds=300) + cb = DelegationCircuitBreaker(config) + assert cb.get_state("a", "b") is CircuitBreakerState.CLOSED + + def test_check_passes_when_closed(self) -> None: + config = CircuitBreakerConfig(bounce_threshold=3, cooldown_seconds=300) + cb = DelegationCircuitBreaker(config) + result = cb.check("a", "b") + assert result.passed is True + assert result.mechanism == "circuit_breaker" + + def test_opens_after_threshold(self) -> None: + config = CircuitBreakerConfig(bounce_threshold=3, cooldown_seconds=300) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + cb = DelegationCircuitBreaker(config, clock=clock) + for _ in range(3): + cb.record_bounce("a", "b") + assert cb.get_state("a", "b") is CircuitBreakerState.OPEN + + def test_check_fails_when_open(self) -> None: + config = CircuitBreakerConfig(bounce_threshold=3, cooldown_seconds=300) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + cb = DelegationCircuitBreaker(config, clock=clock) + for _ in range(3): + cb.record_bounce("a", "b") + result = cb.check("a", "b") + assert result.passed is False + assert result.mechanism == "circuit_breaker" + + def test_resets_after_cooldown(self) -> None: + config = CircuitBreakerConfig(bounce_threshold=3, cooldown_seconds=300) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + cb = DelegationCircuitBreaker(config, clock=clock) + for _ in range(3): + cb.record_bounce("a", "b") + assert cb.get_state("a", "b") is CircuitBreakerState.OPEN + + clock_time = 401.0 # 301s later + assert cb.get_state("a", "b") is CircuitBreakerState.CLOSED + result = cb.check("a", "b") + assert result.passed is True + + def test_sorted_pair_key(self) -> None: + """(a,b) and (b,a) share the same circuit breaker.""" + config = CircuitBreakerConfig(bounce_threshold=2, cooldown_seconds=60) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + cb = DelegationCircuitBreaker(config, clock=clock) + cb.record_bounce("b", "a") + cb.record_bounce("a", "b") + assert cb.get_state("a", "b") is CircuitBreakerState.OPEN + + def test_below_threshold_stays_closed(self) -> None: + config = CircuitBreakerConfig(bounce_threshold=3, cooldown_seconds=300) + cb = DelegationCircuitBreaker(config) + cb.record_bounce("a", "b") + cb.record_bounce("a", "b") + assert cb.get_state("a", "b") is CircuitBreakerState.CLOSED + + def test_different_pair_independent(self) -> None: + config = CircuitBreakerConfig(bounce_threshold=2, cooldown_seconds=60) + cb = DelegationCircuitBreaker(config) + cb.record_bounce("a", "b") + cb.record_bounce("a", "b") + assert cb.get_state("a", "b") is CircuitBreakerState.OPEN + assert cb.get_state("a", "c") is CircuitBreakerState.CLOSED diff --git a/tests/unit/communication/loop_prevention/test_dedup.py b/tests/unit/communication/loop_prevention/test_dedup.py new file mode 100644 index 0000000000..abd67af1e1 --- /dev/null +++ b/tests/unit/communication/loop_prevention/test_dedup.py @@ -0,0 +1,79 @@ +"""Tests for delegation deduplicator.""" + +import pytest + +from ai_company.communication.loop_prevention.dedup import ( + DelegationDeduplicator, +) + +pytestmark = pytest.mark.timeout(30) + + +@pytest.mark.unit +class TestDelegationDeduplicator: + def test_first_check_passes(self) -> None: + dedup = DelegationDeduplicator(window_seconds=60) + result = dedup.check("a", "b", "task-1") + assert result.passed is True + assert result.mechanism == "dedup" + + def test_duplicate_within_window_fails(self) -> None: + clock_time = 100.0 + + def clock() -> float: + return clock_time + + dedup = DelegationDeduplicator(window_seconds=60, clock=clock) + dedup.record("a", "b", "task-1") + clock_time = 130.0 # 30s later, within window + result = dedup.check("a", "b", "task-1") + assert result.passed is False + assert result.mechanism == "dedup" + + def test_duplicate_after_window_passes(self) -> None: + clock_time = 100.0 + + def clock() -> float: + return clock_time + + dedup = DelegationDeduplicator(window_seconds=60, clock=clock) + dedup.record("a", "b", "task-1") + clock_time = 161.0 # 61s later, outside window + result = dedup.check("a", "b", "task-1") + assert result.passed is True + + def test_different_task_title_passes(self) -> None: + clock_time = 100.0 + + def clock() -> float: + return clock_time + + dedup = DelegationDeduplicator(window_seconds=60, clock=clock) + dedup.record("a", "b", "task-1") + result = dedup.check("a", "b", "task-2") + assert result.passed is True + + def test_different_pair_passes(self) -> None: + clock_time = 100.0 + + def clock() -> float: + return clock_time + + dedup = DelegationDeduplicator(window_seconds=60, clock=clock) + dedup.record("a", "b", "task-1") + result = dedup.check("a", "c", "task-1") + assert result.passed is True + + def test_record_updates_timestamp(self) -> None: + clock_time = 100.0 + + def clock() -> float: + return clock_time + + dedup = DelegationDeduplicator(window_seconds=60, clock=clock) + dedup.record("a", "b", "task-1") + clock_time = 150.0 # 50s later + dedup.record("a", "b", "task-1") # re-record + clock_time = 200.0 # 100s after first, 50s after second + result = dedup.check("a", "b", "task-1") + assert result.passed is False # still within window of 2nd diff --git a/tests/unit/communication/loop_prevention/test_depth.py b/tests/unit/communication/loop_prevention/test_depth.py new file mode 100644 index 0000000000..605137b0fb --- /dev/null +++ b/tests/unit/communication/loop_prevention/test_depth.py @@ -0,0 +1,46 @@ +"""Tests for delegation depth check.""" + +import pytest + +from ai_company.communication.loop_prevention.depth import ( + check_delegation_depth, +) + +pytestmark = pytest.mark.timeout(30) + + +@pytest.mark.unit +class TestCheckDelegationDepth: + def test_empty_chain_passes(self) -> None: + result = check_delegation_depth((), max_depth=5) + assert result.passed is True + assert result.mechanism == "max_depth" + + def test_chain_below_limit_passes(self) -> None: + result = check_delegation_depth(("a", "b"), max_depth=5) + assert result.passed is True + + def test_chain_at_limit_fails(self) -> None: + chain = ("a", "b", "c", "d", "e") + result = check_delegation_depth(chain, max_depth=5) + assert result.passed is False + assert result.mechanism == "max_depth" + assert "5" in result.message + + def test_chain_above_limit_fails(self) -> None: + chain = ("a", "b", "c", "d", "e", "f") + result = check_delegation_depth(chain, max_depth=5) + assert result.passed is False + + def test_chain_one_below_limit_passes(self) -> None: + chain = ("a", "b", "c", "d") + result = check_delegation_depth(chain, max_depth=5) + assert result.passed is True + + def test_max_depth_one(self) -> None: + result = check_delegation_depth(("a",), max_depth=1) + assert result.passed is False + + def test_max_depth_one_empty_passes(self) -> None: + result = check_delegation_depth((), max_depth=1) + assert result.passed is True diff --git a/tests/unit/communication/loop_prevention/test_guard.py b/tests/unit/communication/loop_prevention/test_guard.py new file mode 100644 index 0000000000..f313647da0 --- /dev/null +++ b/tests/unit/communication/loop_prevention/test_guard.py @@ -0,0 +1,117 @@ +"""Tests for DelegationGuard orchestrator.""" + +import pytest + +from ai_company.communication.config import ( + CircuitBreakerConfig, + LoopPreventionConfig, + RateLimitConfig, +) +from ai_company.communication.loop_prevention.guard import ( + DelegationGuard, +) + +pytestmark = pytest.mark.timeout(30) + + +def _make_config(**overrides: object) -> LoopPreventionConfig: + """Create a LoopPreventionConfig with test-friendly defaults.""" + defaults: dict[str, object] = { + "max_delegation_depth": 5, + "dedup_window_seconds": 60, + "rate_limit": RateLimitConfig(max_per_pair_per_minute=10, burst_allowance=3), + "circuit_breaker": CircuitBreakerConfig( + bounce_threshold=3, cooldown_seconds=300 + ), + } + defaults.update(overrides) + return LoopPreventionConfig(**defaults) # type: ignore[arg-type] + + +@pytest.mark.unit +class TestDelegationGuard: + def test_all_checks_pass(self) -> None: + guard = DelegationGuard(_make_config()) + result = guard.check( + delegation_chain=("ceo",), + delegator_id="ceo", + delegatee_id="cto", + task_title="Build feature X", + ) + assert result.passed is True + assert result.mechanism == "all_passed" + + def test_ancestry_blocked(self) -> None: + guard = DelegationGuard(_make_config()) + result = guard.check( + delegation_chain=("ceo", "cto"), + delegator_id="cto", + delegatee_id="ceo", + task_title="Build feature X", + ) + assert result.passed is False + assert result.mechanism == "ancestry" + + def test_depth_exceeded(self) -> None: + guard = DelegationGuard(_make_config(max_delegation_depth=2)) + result = guard.check( + delegation_chain=("a", "b"), + delegator_id="b", + delegatee_id="c", + task_title="Task", + ) + assert result.passed is False + assert result.mechanism == "max_depth" + + def test_dedup_blocked(self) -> None: + guard = DelegationGuard(_make_config()) + # First delegation succeeds + result = guard.check((), "a", "b", "Task") + assert result.passed is True + guard.record_delegation("a", "b", "Task") + # Same delegation again + result = guard.check((), "a", "b", "Task") + assert result.passed is False + assert result.mechanism == "dedup" + + def test_rate_limit_blocked(self) -> None: + config = _make_config( + rate_limit=RateLimitConfig(max_per_pair_per_minute=2, burst_allowance=0), + ) + guard = DelegationGuard(config) + for i in range(2): + guard.record_delegation("a", "b", f"Task-{i}") + result = guard.check((), "a", "b", "Task-new") + assert result.passed is False + assert result.mechanism == "rate_limit" + + def test_circuit_breaker_blocked(self) -> None: + config = _make_config( + circuit_breaker=CircuitBreakerConfig( + bounce_threshold=2, cooldown_seconds=300 + ), + ) + guard = DelegationGuard(config) + for i in range(2): + guard.record_delegation("a", "b", f"Task-{i}") + # Circuit should be open after 2 bounces + result = guard.check((), "a", "b", "Task-new-2") + assert result.passed is False + assert result.mechanism == "circuit_breaker" + + def test_record_delegation_records_all(self) -> None: + guard = DelegationGuard(_make_config()) + guard.record_delegation("a", "b", "Task-1") + # After recording, dedup should block same triple + result = guard.check((), "a", "b", "Task-1") + assert result.passed is False + assert result.mechanism == "dedup" + + def test_check_order_ancestry_first(self) -> None: + """Ancestry is checked before depth, so ancestry error wins.""" + config = _make_config(max_delegation_depth=1) + guard = DelegationGuard(config) + # Both ancestry and depth would fail; ancestry checked first + result = guard.check(("a",), "a", "a", "Task") + assert result.passed is False + assert result.mechanism == "ancestry" diff --git a/tests/unit/communication/loop_prevention/test_rate_limit.py b/tests/unit/communication/loop_prevention/test_rate_limit.py new file mode 100644 index 0000000000..f24d49a9ec --- /dev/null +++ b/tests/unit/communication/loop_prevention/test_rate_limit.py @@ -0,0 +1,98 @@ +"""Tests for delegation rate limiter.""" + +import pytest + +from ai_company.communication.config import RateLimitConfig +from ai_company.communication.loop_prevention.rate_limit import ( + DelegationRateLimiter, +) + +pytestmark = pytest.mark.timeout(30) + + +@pytest.mark.unit +class TestDelegationRateLimiter: + def test_first_check_passes(self) -> None: + config = RateLimitConfig(max_per_pair_per_minute=3, burst_allowance=1) + limiter = DelegationRateLimiter(config) + result = limiter.check("a", "b") + assert result.passed is True + assert result.mechanism == "rate_limit" + + def test_within_limit_passes(self) -> None: + config = RateLimitConfig(max_per_pair_per_minute=3, burst_allowance=1) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + limiter = DelegationRateLimiter(config, clock=clock) + # Record 3 delegations (limit = 3 + 1 = 4) + for _ in range(3): + limiter.record("a", "b") + result = limiter.check("a", "b") + assert result.passed is True + + def test_exceeds_limit_fails(self) -> None: + config = RateLimitConfig(max_per_pair_per_minute=3, burst_allowance=1) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + limiter = DelegationRateLimiter(config, clock=clock) + # Record 4 delegations (at limit = 3 + 1 = 4) + for _ in range(4): + limiter.record("a", "b") + result = limiter.check("a", "b") + assert result.passed is False + assert result.mechanism == "rate_limit" + + def test_expired_entries_pruned(self) -> None: + config = RateLimitConfig(max_per_pair_per_minute=2, burst_allowance=0) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + limiter = DelegationRateLimiter(config, clock=clock) + limiter.record("a", "b") + limiter.record("a", "b") + # At this point, limit reached (2 of 2) + result = limiter.check("a", "b") + assert result.passed is False + + # Advance clock past 60s window + clock_time = 161.0 + result = limiter.check("a", "b") + assert result.passed is True + + def test_sorted_pair_key(self) -> None: + """(a,b) and (b,a) share the same rate limit.""" + config = RateLimitConfig(max_per_pair_per_minute=2, burst_allowance=0) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + limiter = DelegationRateLimiter(config, clock=clock) + limiter.record("b", "a") # reversed order + limiter.record("a", "b") # normal order + result = limiter.check("a", "b") + assert result.passed is False + + def test_different_pair_independent(self) -> None: + config = RateLimitConfig(max_per_pair_per_minute=1, burst_allowance=0) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + limiter = DelegationRateLimiter(config, clock=clock) + limiter.record("a", "b") + # Pair (a,b) is at limit + result = limiter.check("a", "b") + assert result.passed is False + # Pair (a,c) is unaffected + result = limiter.check("a", "c") + assert result.passed is True diff --git a/tests/unit/communication/test_errors.py b/tests/unit/communication/test_errors.py index 36f4a4fc2c..38451849f9 100644 --- a/tests/unit/communication/test_errors.py +++ b/tests/unit/communication/test_errors.py @@ -6,11 +6,22 @@ ChannelAlreadyExistsError, ChannelNotFoundError, CommunicationError, + DelegationAncestryError, + DelegationAuthorityError, + DelegationCircuitOpenError, + DelegationDepthError, + DelegationDuplicateError, + DelegationError, + DelegationLoopError, + DelegationRateLimitError, + HierarchyResolutionError, MessageBusAlreadyRunningError, MessageBusNotRunningError, NotSubscribedError, ) +pytestmark = pytest.mark.timeout(30) + class TestCommunicationError: """Tests for the base CommunicationError.""" @@ -62,6 +73,15 @@ class TestSubclasses: (NotSubscribedError, CommunicationError), (MessageBusNotRunningError, CommunicationError), (MessageBusAlreadyRunningError, CommunicationError), + (DelegationError, CommunicationError), + (DelegationAuthorityError, DelegationError), + (DelegationLoopError, DelegationError), + (DelegationDepthError, DelegationLoopError), + (DelegationAncestryError, DelegationLoopError), + (DelegationRateLimitError, DelegationLoopError), + (DelegationCircuitOpenError, DelegationLoopError), + (DelegationDuplicateError, DelegationLoopError), + (HierarchyResolutionError, CommunicationError), ], ) def test_inherits_base(self, cls: type, base: type) -> None: @@ -76,9 +96,34 @@ def test_inherits_base(self, cls: type, base: type) -> None: NotSubscribedError, MessageBusNotRunningError, MessageBusAlreadyRunningError, + DelegationError, + DelegationAuthorityError, + DelegationLoopError, + DelegationDepthError, + DelegationAncestryError, + DelegationRateLimitError, + DelegationCircuitOpenError, + DelegationDuplicateError, + HierarchyResolutionError, ], ) def test_subclass_carries_context(self, cls: type) -> None: err = cls("msg", context={"k": "v"}) assert err.context["k"] == "v" assert isinstance(err, CommunicationError) + + +@pytest.mark.unit +class TestDelegationErrorHierarchy: + """Tests for delegation error inheritance chain.""" + + def test_depth_error_chain(self) -> None: + err = DelegationDepthError("too deep") + assert isinstance(err, DelegationLoopError) + assert isinstance(err, DelegationError) + assert isinstance(err, CommunicationError) + + def test_hierarchy_error_is_communication_error(self) -> None: + err = HierarchyResolutionError("broken") + assert isinstance(err, CommunicationError) + assert not isinstance(err, DelegationError) diff --git a/tests/unit/core/test_task.py b/tests/unit/core/test_task.py index 228747f88e..6201743de5 100644 --- a/tests/unit/core/test_task.py +++ b/tests/unit/core/test_task.py @@ -274,6 +274,63 @@ def test_non_active_allows_assigned_to(self, status: TaskStatus) -> None: assert task.status is status +# ── Task: Delegation Fields ───────────────────────────────────── + + +@pytest.mark.unit +class TestTaskDelegationFields: + def test_delegation_defaults(self) -> None: + task = _make_task() + assert task.parent_task_id is None + assert task.delegation_chain == () + + def test_parent_task_id_set(self) -> None: + task = _make_task(parent_task_id="parent-1") + assert task.parent_task_id == "parent-1" + + def test_parent_task_id_equals_id_rejected(self) -> None: + with pytest.raises(ValidationError, match="cannot be its own parent"): + _make_task(parent_task_id="task-123") + + def test_delegation_chain_set(self) -> None: + task = _make_task(delegation_chain=("a", "b", "c")) + assert task.delegation_chain == ("a", "b", "c") + + def test_delegation_chain_duplicates_rejected(self) -> None: + with pytest.raises( + ValidationError, match="Duplicate entries in delegation_chain" + ): + _make_task(delegation_chain=("a", "b", "a")) + + def test_delegation_chain_contains_assigned_to_rejected(self) -> None: + with pytest.raises( + ValidationError, match="must not appear in delegation_chain" + ): + _make_task( + assigned_to="agent-1", + delegation_chain=("agent-1", "agent-2"), + status=TaskStatus.ASSIGNED, + ) + + def test_delegation_chain_without_assigned_to_overlap_ok(self) -> None: + task = _make_task( + delegation_chain=("a", "b"), + assigned_to="c", + status=TaskStatus.ASSIGNED, + ) + assert task.delegation_chain == ("a", "b") + assert task.assigned_to == "c" + + def test_serialization_includes_delegation_fields(self) -> None: + task = _make_task( + parent_task_id="parent-1", + delegation_chain=("x", "y"), + ) + dumped = task.model_dump() + assert dumped["parent_task_id"] == "parent-1" + assert dumped["delegation_chain"] == ("x", "y") + + # ── Task: Max Retries ─────────────────────────────────────────── diff --git a/tests/unit/observability/test_events.py b/tests/unit/observability/test_events.py index 8867614b5d..27d9be845c 100644 --- a/tests/unit/observability/test_events.py +++ b/tests/unit/observability/test_events.py @@ -106,6 +106,7 @@ def test_all_domain_modules_discovered(self) -> None: "company", "config", "correlation", + "delegation", "execution", "git", "personality", From d46b4556ddcb270c0672b33558e5ef0437f7ae90 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sat, 7 Mar 2026 19:42:01 +0100 Subject: [PATCH 2/4] refactor: address pre-PR review findings across delegation and loop prevention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-reviewed by 9 agents, 26 findings addressed: - Extract shared _pair_key utility (DRY violation) - Add model validators to DelegationResult, AuthorityCheckResult, GuardCheckOutcome - Fix circuit breaker re-arming bug in record_bounce - Add O(n) DFS cycle detection replacing O(n²) approach - Fix duplicate entries in hierarchy reporting lines - Freeze hierarchy internal state with MappingProxyType - Add logging to ancestry and depth check modules - Add circuit reset event logging - Prune expired entries in dedup and rate limiter - Split guard checks into pure/stateful phases - Rename create_sub_task to _create_sub_task (private) - Update DESIGN_SPEC.md, CLAUDE.md, README.md Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 2 +- DESIGN_SPEC.md | 22 +++++- README.md | 3 +- .../communication/delegation/authority.py | 15 +++- .../communication/delegation/hierarchy.py | 56 ++++++++++----- .../communication/delegation/models.py | 22 +++++- .../communication/delegation/service.py | 4 +- .../loop_prevention/_pair_key.py | 22 ++++++ .../communication/loop_prevention/ancestry.py | 11 +++ .../loop_prevention/circuit_breaker.py | 44 ++++++++---- .../communication/loop_prevention/dedup.py | 6 +- .../communication/loop_prevention/depth.py | 11 +++ .../communication/loop_prevention/guard.py | 51 +++++++++---- .../communication/loop_prevention/models.py | 15 +++- .../loop_prevention/rate_limit.py | 31 ++++---- src/ai_company/core/task.py | 7 +- .../observability/events/delegation.py | 1 + .../delegation/test_hierarchy.py | 72 +++++++++++++++++++ .../communication/delegation/test_models.py | 29 +++++++- .../loop_prevention/test_dedup.py | 27 +++++++ 20 files changed, 379 insertions(+), 72 deletions(-) create mode 100644 src/ai_company/communication/loop_prevention/_pair_key.py diff --git a/CLAUDE.md b/CLAUDE.md index 878cd642ff..33396e12a7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,7 +46,7 @@ src/ai_company/ api/ # FastAPI REST + WebSocket routes budget/ # Per-agent cost tracking and spending controls cli/ # Typer CLI commands - communication/ # Message bus (protocol + in-memory backend), dispatcher, messenger facade, channels + communication/ # Message bus, dispatcher, messenger, channels, delegation, loop prevention config/ # YAML company config loading and validation core/ # Shared domain models and base classes engine/ # Agent orchestration, execution loops, task lifecycle, recovery, and shutdown diff --git a/DESIGN_SPEC.md b/DESIGN_SPEC.md index 65b388c3b7..95c3fb976f 100644 --- a/DESIGN_SPEC.md +++ b/DESIGN_SPEC.md @@ -578,7 +578,7 @@ When a loop is detected, the framework: 3. Escalates to the sender's manager (or human if at top of hierarchy) 4. Logs the loop for analytics and process improvement -> **Current state (M4 in-progress):** The communication foundation is implemented: `MessageBus` protocol with `InMemoryMessageBus` backend (asyncio queues, pull-model `receive()`), `MessageDispatcher` for concurrent handler routing via `asyncio.TaskGroup`, `AgentMessenger` per-agent facade (auto-fills sender/timestamp/ID, deterministic direct-channel naming `@{sorted_a}:{sorted_b}`), and `DeliveryEnvelope` for delivery tracking. Loop prevention (§5.5), conflict resolution (§5.6), and meeting protocol (§5.7) are planned for later M4 work. +> **Current state (M4 in-progress):** The communication foundation is implemented: `MessageBus` protocol with `InMemoryMessageBus` backend (asyncio queues, pull-model `receive()`), `MessageDispatcher` for concurrent handler routing via `asyncio.TaskGroup`, `AgentMessenger` per-agent facade (auto-fills sender/timestamp/ID, deterministic direct-channel naming `@{sorted_a}:{sorted_b}`), and `DeliveryEnvelope` for delivery tracking. Loop prevention (§5.5) is implemented: `DelegationGuard` orchestrates five mechanisms (ancestry, depth, dedup, rate limit, circuit breaker) with `LoopPreventionConfig`. Hierarchical delegation is implemented via `DelegationService` with `HierarchyResolver` and `AuthorityValidator`. Task model extended with `parent_task_id` and `delegation_chain` fields. Conflict resolution (§5.6) and meeting protocol (§5.7) are planned for later M4 work. ### 5.6 Conflict Resolution Protocol @@ -785,6 +785,8 @@ task: deadline: null max_retries: 1 # max reassignment attempts after failure (0 = no retry) status: "assigned" + parent_task_id: null # parent task ID when created via delegation + delegation_chain: [] # ordered agent IDs of delegators (root first) ``` ### 6.3 Workflow Types @@ -2350,10 +2352,24 @@ ai-company/ │ │ ├── bus_protocol.py # MessageBus protocol interface │ │ ├── channel.py # Channel model │ │ ├── config.py # Communication config +│ │ ├── delegation/ # Hierarchical delegation subsystem +│ │ │ ├── authority.py # AuthorityValidator + AuthorityCheckResult +│ │ │ ├── hierarchy.py # HierarchyResolver (org hierarchy from Company) +│ │ │ ├── models.py # DelegationRequest, DelegationResult, DelegationRecord +│ │ │ └── service.py # DelegationService (orchestrates delegation flow) │ │ ├── dispatcher.py # MessageDispatcher + DispatchResult │ │ ├── enums.py # Communication enums -│ │ ├── errors.py # Communication error hierarchy +│ │ ├── errors.py # Communication + delegation error hierarchy │ │ ├── handler.py # MessageHandler protocol, FunctionHandler, HandlerRegistration +│ │ ├── loop_prevention/ # Delegation loop prevention mechanisms +│ │ │ ├── _pair_key.py # Canonical agent-pair key utility +│ │ │ ├── ancestry.py # Ancestry cycle detection (pure function) +│ │ │ ├── circuit_breaker.py # DelegationCircuitBreaker (per-pair) +│ │ │ ├── dedup.py # DelegationDeduplicator (time-windowed) +│ │ │ ├── depth.py # Max delegation depth check (pure function) +│ │ │ ├── guard.py # DelegationGuard (orchestrates all mechanisms) +│ │ │ ├── models.py # GuardCheckOutcome, CircuitBreakerState +│ │ │ └── rate_limit.py # DelegationRateLimiter (per-pair) │ │ ├── message.py # Message model │ │ ├── messenger.py # AgentMessenger per-agent facade │ │ └── subscription.py # Subscription + DeliveryEnvelope models @@ -2373,6 +2389,7 @@ ai-company/ │ │ │ ├── budget.py # BUDGET_* constants │ │ │ ├── communication.py # COMM_* constants │ │ │ ├── config.py # CONFIG_* constants +│ │ │ ├── delegation.py # DELEGATION_* constants │ │ │ ├── correlation.py # CORRELATION_* constants │ │ │ ├── execution.py # EXECUTION_* constants │ │ │ ├── git.py # GIT_* constants @@ -2532,6 +2549,7 @@ These conventions were established during the M0–M2+ review cycle. **Adopted** | **Workspace isolation** | Planned (M4) | Pluggable `WorkspaceIsolationStrategy` protocol. Default: planner + git worktrees. Each agent works in an isolated worktree; sequential merge on completion. Textual conflicts detected by git; semantic conflicts reviewed by agent or human. | Industry standard (Codex, Cursor, Claude Code, VS Code). Maximum parallelism. Leverages mature git infrastructure. See §6.8. | | **Graceful shutdown** | Adopted (M3) | Pluggable `ShutdownStrategy` protocol. Default: cooperative with 30s timeout. Agents check shutdown event at turn boundaries. Force-cancel after timeout. `INTERRUPTED` status for force-cancelled tasks. M4/M5: upgrade to checkpoint-and-stop. | Cross-platform (Windows `signal.signal()` fallback). Bounded shutdown time. Mirrors cooperative shutdown in §6.7. | | **Communication foundation** | Adopted (M4) | `MessageBus` protocol with `InMemoryMessageBus` backend (asyncio queues, pull-model `receive()` with shutdown signaling via `asyncio.Event`). `MessageDispatcher` routes to concurrent handlers via `asyncio.TaskGroup` with pre-allocated error collection. `AgentMessenger` per-agent facade auto-fills sender/timestamp/ID; deterministic direct-channel naming `@{sorted_a}:{sorted_b}`. `DeliveryEnvelope` for delivery tracking. `NotBlankStr` validation on all protocol boundary identifiers. | Pull-model avoids callback complexity and enables agents to consume at their own pace. Protocol + backend split enables future persistent/distributed bus implementations. Deterministic DM channel names prevent duplicates. See §5. | +| **Delegation & loop prevention** | Adopted (M4) | `HierarchyResolver` resolves org hierarchy from `Company` at construction (cycle-detected, `MappingProxyType`-frozen). `AuthorityValidator` checks chain-of-command + role permissions. `DelegationGuard` orchestrates five mechanisms (ancestry, depth, dedup, rate limit, circuit breaker) in sequence, short-circuiting on first rejection. `DelegationService` is synchronous (CPU-only); messaging integration deferred. Stateful mechanisms use injectable clock for deterministic testing. Task model extended with `parent_task_id` and `delegation_chain` fields. | Synchronous delegation avoids async complexity for CPU-only validation. Five-mechanism guard provides defence-in-depth against all loop patterns. Injectable clocks enable deterministic testing. See §5.4, §5.5. | --- diff --git a/README.md b/README.md index 152eb36a81..37f0112b33 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,7 @@ AI Company lets you spin up a virtual organization staffed entirely by AI agents - **Deep Agent Identity** - Names, personalities, skills, seniority levels, performance tracking - **Multi-Provider** - Any LLM via LiteLLM — cloud APIs, OpenRouter (400+ models), local Ollama, and more - **Smart Cost Management** - Per-agent budget tracking, auto model routing, CFO agent optimization +- **Hierarchical Delegation** - Chain-of-command task delegation with five-mechanism loop prevention - **Configurable Autonomy** - From fully autonomous to human-approves-everything, with a Security Ops agent in between - **Persistent Memory** - Agents remember past decisions, code, relationships (memory layer TBD) - **HR System** - Hire, fire, promote agents. HR agent analyzes skill gaps and proposes candidates @@ -23,7 +24,7 @@ AI Company lets you spin up a virtual organization staffed entirely by AI agents ## Status -**M3: Single Agent** in progress (M0 Tooling, M1 Config & Core, M2 Providers — all done). See [DESIGN_SPEC.md](DESIGN_SPEC.md) for the full high-level specification. +**M4: Multi-Agent** in progress (M0 Tooling, M1 Config & Core, M2 Providers, M3 Single Agent — all done). See [DESIGN_SPEC.md](DESIGN_SPEC.md) for the full high-level specification. ## Tech Stack diff --git a/src/ai_company/communication/delegation/authority.py b/src/ai_company/communication/delegation/authority.py index 73ff6fcd87..6e711f004d 100644 --- a/src/ai_company/communication/delegation/authority.py +++ b/src/ai_company/communication/delegation/authority.py @@ -1,6 +1,8 @@ """Authority validation for hierarchical delegation.""" -from pydantic import BaseModel, ConfigDict, Field +from typing import Self + +from pydantic import BaseModel, ConfigDict, Field, model_validator from ai_company.communication.config import HierarchyConfig # noqa: TC001 from ai_company.communication.delegation.hierarchy import ( # noqa: TC001 @@ -29,6 +31,17 @@ class AuthorityCheckResult(BaseModel): allowed: bool = Field(description="Whether delegation is allowed") reason: str = Field(default="", description="Explanation") + @model_validator(mode="after") + def _validate_allowed_reason(self) -> Self: + """Enforce allowed/reason correlation.""" + if self.allowed and self.reason: + msg = "reason must be empty when allowed is True" + raise ValueError(msg) + if not self.allowed and not self.reason: + msg = "reason is required when allowed is False" + raise ValueError(msg) + return self + class AuthorityValidator: """Validates delegation authority using hierarchy and role permissions. diff --git a/src/ai_company/communication/delegation/hierarchy.py b/src/ai_company/communication/delegation/hierarchy.py index 3a6fc44957..11f3e926ae 100644 --- a/src/ai_company/communication/delegation/hierarchy.py +++ b/src/ai_company/communication/delegation/hierarchy.py @@ -1,5 +1,7 @@ """Hierarchy resolver for organizational structure.""" +from types import MappingProxyType + from ai_company.communication.errors import HierarchyResolutionError from ai_company.core.company import Company # noqa: TC001 from ai_company.observability import get_logger @@ -16,7 +18,8 @@ class HierarchyResolver: Built from three sources, in priority order: - 1. Explicit ``ReportingLine.supervisor`` (most specific) + 1. Explicit ``ReportingLine.supervisor`` (most specific) — overrides + team-derived relationships. 2. ``Team.lead`` for team members 3. ``Department.head`` for team leads without explicit reporting @@ -37,12 +40,12 @@ def __init__(self, company: Company) -> None: for dept in company.departments: for team in dept.teams: - # Team lead → department head (lowest priority) + # Team lead -> department head (lowest priority) if team.lead not in supervisor_of: supervisor_of[team.lead] = dept.head reports_of.setdefault(dept.head, []).append(team.lead) - # Team members → team lead (medium priority) + # Team members -> team lead (medium priority) for member in team.members: if member == team.lead: continue @@ -53,22 +56,29 @@ def __init__(self, company: Company) -> None: # Explicit reporting lines (highest priority — override) for line in dept.reporting_lines: old_sup = supervisor_of.get(line.subordinate) - if old_sup is not None and old_sup != line.supervisor: + if old_sup == line.supervisor: + continue + if old_sup is not None: # Remove from old supervisor's reports old_reports = reports_of.get(old_sup, []) - if line.subordinate in old_reports: - old_reports.remove(line.subordinate) + reports_of[old_sup] = [ + r for r in old_reports if r != line.subordinate + ] supervisor_of[line.subordinate] = line.supervisor - reports_of.setdefault(line.supervisor, []).append(line.subordinate) + reports_of.setdefault(line.supervisor, []).append( + line.subordinate, + ) # Cycle detection self._detect_cycles(supervisor_of) - # Freeze internal state - self._supervisor_of: dict[str, str] = supervisor_of - self._reports_of: dict[str, tuple[str, ...]] = { - k: tuple(v) for k, v in reports_of.items() - } + # Freeze internal state with MappingProxyType + self._supervisor_of: MappingProxyType[str, str] = MappingProxyType( + supervisor_of, + ) + self._reports_of: MappingProxyType[str, tuple[str, ...]] = MappingProxyType( + {k: tuple(v) for k, v in reports_of.items()} + ) logger.debug( DELEGATION_HIERARCHY_BUILT, @@ -78,7 +88,10 @@ def __init__(self, company: Company) -> None: @staticmethod def _detect_cycles(supervisor_of: dict[str, str]) -> None: - """Detect cycles in the supervisor graph. + """Detect cycles in the supervisor graph via single-pass DFS. + + Uses a visited/in-stack colouring approach for O(n) + complexity instead of restarting traversal per node. Args: supervisor_of: Mapping from agent to supervisor. @@ -86,11 +99,15 @@ def _detect_cycles(supervisor_of: dict[str, str]) -> None: Raises: HierarchyResolutionError: If a cycle is found. """ + visited: set[str] = set() + for agent in supervisor_of: - visited: set[str] = set() - current = agent - while current in supervisor_of: - if current in visited: + if agent in visited: + continue + in_stack: set[str] = set() + current: str | None = agent + while current is not None and current not in visited: + if current in in_stack: logger.warning( DELEGATION_HIERARCHY_CYCLE, agent=agent, @@ -107,8 +124,9 @@ def _detect_cycles(supervisor_of: dict[str, str]) -> None: "cycle_at": current, }, ) - visited.add(current) - current = supervisor_of[current] + in_stack.add(current) + current = supervisor_of.get(current) + visited.update(in_stack) def get_supervisor(self, agent_name: str) -> str | None: """Get the direct supervisor of an agent. diff --git a/src/ai_company/communication/delegation/models.py b/src/ai_company/communication/delegation/models.py index e4da3a5053..861855d91e 100644 --- a/src/ai_company/communication/delegation/models.py +++ b/src/ai_company/communication/delegation/models.py @@ -1,6 +1,8 @@ """Delegation request, result, and audit trail models.""" -from pydantic import AwareDatetime, BaseModel, ConfigDict, Field +from typing import Self + +from pydantic import AwareDatetime, BaseModel, ConfigDict, Field, model_validator from ai_company.core.task import Task # noqa: TC001 from ai_company.core.types import NotBlankStr # noqa: TC001 @@ -62,6 +64,24 @@ class DelegationResult(BaseModel): description="Mechanism name that blocked delegation", ) + @model_validator(mode="after") + def _validate_success_consistency(self) -> Self: + """Enforce success/failure field correlation.""" + if self.success: + if self.delegated_task is None: + msg = "delegated_task is required when success is True" + raise ValueError(msg) + if self.rejection_reason is not None: + msg = "rejection_reason must be None when success is True" + raise ValueError(msg) + if self.blocked_by is not None: + msg = "blocked_by must be None when success is True" + raise ValueError(msg) + elif self.rejection_reason is None: + msg = "rejection_reason is required when success is False" + raise ValueError(msg) + return self + class DelegationRecord(BaseModel): """Audit trail entry for a completed delegation. diff --git a/src/ai_company/communication/delegation/service.py b/src/ai_company/communication/delegation/service.py index abc325fd23..bde05fe8d9 100644 --- a/src/ai_company/communication/delegation/service.py +++ b/src/ai_company/communication/delegation/service.py @@ -107,7 +107,7 @@ def delegate( ) # 3. Create sub-task - sub_task = self.create_sub_task(request) + sub_task = self._create_sub_task(request) # 4. Record in guard and audit trail self._guard.record_delegation( @@ -139,7 +139,7 @@ def delegate( delegated_task=sub_task, ) - def create_sub_task(self, request: DelegationRequest) -> Task: + def _create_sub_task(self, request: DelegationRequest) -> Task: """Create a new sub-task from a delegation request. The sub-task inherits the original task's properties but gets diff --git a/src/ai_company/communication/loop_prevention/_pair_key.py b/src/ai_company/communication/loop_prevention/_pair_key.py new file mode 100644 index 0000000000..7f2244135a --- /dev/null +++ b/src/ai_company/communication/loop_prevention/_pair_key.py @@ -0,0 +1,22 @@ +"""Canonical agent-pair key utility. + +Both ``DelegationCircuitBreaker`` and ``DelegationRateLimiter`` track +state per *undirected* agent pair. This helper normalises the key so +that ``(a, b)`` and ``(b, a)`` map to the same entry. +""" + + +def pair_key(a: str, b: str) -> tuple[str, str]: + """Create a canonical sorted key for an agent pair. + + The key is direction-agnostic: ``pair_key("x", "y")`` equals + ``pair_key("y", "x")``. + + Args: + a: First agent ID. + b: Second agent ID. + + Returns: + Lexicographically sorted ``(min, max)`` tuple. + """ + return (min(a, b), max(a, b)) diff --git a/src/ai_company/communication/loop_prevention/ancestry.py b/src/ai_company/communication/loop_prevention/ancestry.py index 5157ab6981..c0245f626a 100644 --- a/src/ai_company/communication/loop_prevention/ancestry.py +++ b/src/ai_company/communication/loop_prevention/ancestry.py @@ -1,6 +1,12 @@ """Ancestry cycle detection check (pure function).""" from ai_company.communication.loop_prevention.models import GuardCheckOutcome +from ai_company.observability import get_logger +from ai_company.observability.events.delegation import ( + DELEGATION_LOOP_ANCESTRY_BLOCKED, +) + +logger = get_logger(__name__) _MECHANISM = "ancestry" @@ -19,6 +25,11 @@ def check_ancestry( Outcome with passed=False if delegatee is already in the chain. """ if delegatee_id in delegation_chain: + logger.info( + DELEGATION_LOOP_ANCESTRY_BLOCKED, + delegatee=delegatee_id, + chain=delegation_chain, + ) return GuardCheckOutcome( passed=False, mechanism=_MECHANISM, diff --git a/src/ai_company/communication/loop_prevention/circuit_breaker.py b/src/ai_company/communication/loop_prevention/circuit_breaker.py index 834b075d47..99ef49ec58 100644 --- a/src/ai_company/communication/loop_prevention/circuit_breaker.py +++ b/src/ai_company/communication/loop_prevention/circuit_breaker.py @@ -5,10 +5,12 @@ from enum import StrEnum from ai_company.communication.config import CircuitBreakerConfig # noqa: TC001 +from ai_company.communication.loop_prevention._pair_key import pair_key from ai_company.communication.loop_prevention.models import GuardCheckOutcome from ai_company.observability import get_logger from ai_company.observability.events.delegation import ( DELEGATION_LOOP_CIRCUIT_OPEN, + DELEGATION_LOOP_CIRCUIT_RESET, ) logger = get_logger(__name__) @@ -28,13 +30,13 @@ class CircuitBreakerState(StrEnum): OPEN = "open" -def _pair_key(a: str, b: str) -> tuple[str, str]: - """Create a canonical sorted key for an agent pair.""" - return (min(a, b), max(a, b)) - - class _PairState: - """Internal mutable state for a single agent pair.""" + """Internal mutable state for a single agent pair. + + Attributes: + bounce_count: Delegations since last reset. + opened_at: Monotonic timestamp when opened, or ``None``. + """ __slots__ = ("bounce_count", "opened_at") @@ -72,10 +74,8 @@ def _get_pair( delegator_id: str, delegatee_id: str, ) -> _PairState: - key = _pair_key(delegator_id, delegatee_id) - if key not in self._pairs: - self._pairs[key] = _PairState() - return self._pairs[key] + key = pair_key(delegator_id, delegatee_id) + return self._pairs.setdefault(key, _PairState()) def get_state( self, @@ -84,6 +84,9 @@ def get_state( ) -> CircuitBreakerState: """Get the circuit breaker state for an agent pair. + If the circuit was previously open and the cooldown has expired, + the pair state is reset before returning ``CLOSED``. + Args: delegator_id: First agent ID. delegatee_id: Second agent ID. @@ -97,6 +100,12 @@ def get_state( if elapsed < self._config.cooldown_seconds: return CircuitBreakerState.OPEN # Cooldown expired: reset + logger.info( + DELEGATION_LOOP_CIRCUIT_RESET, + delegator=delegator_id, + delegatee=delegatee_id, + cooldown_seconds=self._config.cooldown_seconds, + ) pair.bounce_count = 0 pair.opened_at = None return CircuitBreakerState.CLOSED @@ -141,14 +150,25 @@ def record_bounce( """Record a delegation bounce for the pair. If the bounce count reaches the threshold, opens the circuit. + If the circuit was previously open and the cooldown has expired, + the bounce count is reset before recording. If the circuit is + already open (cooldown not yet expired), this call is a no-op. Args: delegator_id: First agent ID. delegatee_id: Second agent ID. """ + state = self.get_state(delegator_id, delegatee_id) + if state is CircuitBreakerState.OPEN: + return pair = self._get_pair(delegator_id, delegatee_id) - # If cooldown expired, get_state already reset - self.get_state(delegator_id, delegatee_id) pair.bounce_count += 1 if pair.bounce_count >= self._config.bounce_threshold: pair.opened_at = self._clock() + logger.warning( + DELEGATION_LOOP_CIRCUIT_OPEN, + delegator=delegator_id, + delegatee=delegatee_id, + bounce_count=pair.bounce_count, + threshold=self._config.bounce_threshold, + ) diff --git a/src/ai_company/communication/loop_prevention/dedup.py b/src/ai_company/communication/loop_prevention/dedup.py index 58122d6d43..4002796879 100644 --- a/src/ai_company/communication/loop_prevention/dedup.py +++ b/src/ai_company/communication/loop_prevention/dedup.py @@ -50,11 +50,15 @@ def check( Returns: Outcome with passed=False if a duplicate exists. """ + # Directional key: A->B and B->A are distinct delegations key = (delegator_id, delegatee_id, task_title) recorded_at = self._records.get(key) if recorded_at is not None: elapsed = self._clock() - recorded_at - if elapsed < self._window_seconds: + if elapsed >= self._window_seconds: + # Expired: remove stale entry + del self._records[key] + else: logger.info( DELEGATION_LOOP_DEDUP_BLOCKED, delegator=delegator_id, diff --git a/src/ai_company/communication/loop_prevention/depth.py b/src/ai_company/communication/loop_prevention/depth.py index c85c22611e..6c262df63d 100644 --- a/src/ai_company/communication/loop_prevention/depth.py +++ b/src/ai_company/communication/loop_prevention/depth.py @@ -1,6 +1,12 @@ """Max delegation depth check (pure function).""" from ai_company.communication.loop_prevention.models import GuardCheckOutcome +from ai_company.observability import get_logger +from ai_company.observability.events.delegation import ( + DELEGATION_LOOP_DEPTH_EXCEEDED, +) + +logger = get_logger(__name__) _MECHANISM = "max_depth" @@ -19,6 +25,11 @@ def check_delegation_depth( Outcome with passed=True if within limit. """ if len(delegation_chain) >= max_depth: + logger.info( + DELEGATION_LOOP_DEPTH_EXCEEDED, + chain_length=len(delegation_chain), + max_depth=max_depth, + ) return GuardCheckOutcome( passed=False, mechanism=_MECHANISM, diff --git a/src/ai_company/communication/loop_prevention/guard.py b/src/ai_company/communication/loop_prevention/guard.py index 3fea0934db..a7110b1c5a 100644 --- a/src/ai_company/communication/loop_prevention/guard.py +++ b/src/ai_company/communication/loop_prevention/guard.py @@ -26,10 +26,10 @@ class DelegationGuard: - """Orchestrates all five loop prevention mechanisms. + """Orchestrates all loop prevention mechanisms. Checks run in order: ancestry, depth, dedup, rate_limit, - circuit_breaker. Returns the first failure or an overall success. + circuit_breaker. Short-circuits on the first failure. Args: config: Loop prevention configuration. @@ -73,28 +73,49 @@ def check( Returns: First failing outcome or an all-passed success. """ - checks = [ + # Pure (stateless) checks first + for outcome in ( check_ancestry(delegation_chain, delegatee_id), - check_delegation_depth(delegation_chain, self._config.max_delegation_depth), - self._deduplicator.check(delegator_id, delegatee_id, task_title), + check_delegation_depth( + delegation_chain, + self._config.max_delegation_depth, + ), + ): + if not outcome.passed: + return self._log_and_return(outcome, delegator_id, delegatee_id) + # Stateful checks — only run if pure checks passed + for outcome in ( + self._deduplicator.check( + delegator_id, + delegatee_id, + task_title, + ), self._rate_limiter.check(delegator_id, delegatee_id), self._circuit_breaker.check(delegator_id, delegatee_id), - ] - for outcome in checks: + ): if not outcome.passed: - logger.info( - DELEGATION_LOOP_BLOCKED, - mechanism=outcome.mechanism, - delegator=delegator_id, - delegatee=delegatee_id, - message=outcome.message, - ) - return outcome + return self._log_and_return(outcome, delegator_id, delegatee_id) return GuardCheckOutcome( passed=True, mechanism=_SUCCESS_MECHANISM, ) + @staticmethod + def _log_and_return( + outcome: GuardCheckOutcome, + delegator_id: str, + delegatee_id: str, + ) -> GuardCheckOutcome: + """Log a blocked delegation and return the outcome.""" + logger.info( + DELEGATION_LOOP_BLOCKED, + mechanism=outcome.mechanism, + delegator=delegator_id, + delegatee=delegatee_id, + message=outcome.message, + ) + return outcome + def record_delegation( self, delegator_id: str, diff --git a/src/ai_company/communication/loop_prevention/models.py b/src/ai_company/communication/loop_prevention/models.py index cdfd14fbfc..c3c4c1d4bd 100644 --- a/src/ai_company/communication/loop_prevention/models.py +++ b/src/ai_company/communication/loop_prevention/models.py @@ -1,6 +1,8 @@ """Loop prevention check outcome model.""" -from pydantic import BaseModel, ConfigDict, Field +from typing import Self + +from pydantic import BaseModel, ConfigDict, Field, model_validator from ai_company.core.types import NotBlankStr # noqa: TC001 @@ -24,3 +26,14 @@ class GuardCheckOutcome(BaseModel): default="", description="Human-readable detail", ) + + @model_validator(mode="after") + def _validate_passed_message(self) -> Self: + """Enforce passed/message correlation.""" + if self.passed and self.message: + msg = "message must be empty when passed is True" + raise ValueError(msg) + if not self.passed and not self.message: + msg = "message is required when passed is False" + raise ValueError(msg) + return self diff --git a/src/ai_company/communication/loop_prevention/rate_limit.py b/src/ai_company/communication/loop_prevention/rate_limit.py index 8fb602bcc9..61183e4655 100644 --- a/src/ai_company/communication/loop_prevention/rate_limit.py +++ b/src/ai_company/communication/loop_prevention/rate_limit.py @@ -4,6 +4,7 @@ from collections.abc import Callable # noqa: TC003 from ai_company.communication.config import RateLimitConfig # noqa: TC001 +from ai_company.communication.loop_prevention._pair_key import pair_key from ai_company.communication.loop_prevention.models import GuardCheckOutcome from ai_company.observability import get_logger from ai_company.observability.events.delegation import ( @@ -13,36 +14,35 @@ logger = get_logger(__name__) _MECHANISM = "rate_limit" -_WINDOW_SECONDS = 60.0 - - -def _pair_key(a: str, b: str) -> tuple[str, str]: - """Create a canonical sorted key for an agent pair.""" - return (min(a, b), max(a, b)) +_DEFAULT_WINDOW_SECONDS = 60.0 class DelegationRateLimiter: """Per-pair rate limit with burst allowance. The key is the sorted (a, b) agent pair. Counts delegations within - the last 60-second window. If the count exceeds + the sliding window. If the count exceeds ``max_per_pair_per_minute + burst_allowance``, the check fails. Args: config: Rate limit configuration. clock: Monotonic clock function for deterministic testing. + window_seconds: Duration of the sliding window. Defaults to + 60.0, matching the ``max_per_pair_per_minute`` semantics. """ - __slots__ = ("_clock", "_config", "_timestamps") + __slots__ = ("_clock", "_config", "_timestamps", "_window_seconds") def __init__( self, config: RateLimitConfig, *, clock: Callable[[], float] = time.monotonic, + window_seconds: float = _DEFAULT_WINDOW_SECONDS, ) -> None: self._config = config self._clock = clock + self._window_seconds = window_seconds self._timestamps: dict[tuple[str, str], list[float]] = {} def check( @@ -52,6 +52,8 @@ def check( ) -> GuardCheckOutcome: """Check whether the pair has exceeded the rate limit. + Expired timestamps are pruned on every call. + Args: delegator_id: ID of the delegating agent. delegatee_id: ID of the target agent. @@ -59,11 +61,13 @@ def check( Returns: Outcome with passed=False if rate limit exceeded. """ - key = _pair_key(delegator_id, delegatee_id) + key = pair_key(delegator_id, delegatee_id) now = self._clock() - cutoff = now - _WINDOW_SECONDS + cutoff = now - self._window_seconds timestamps = self._timestamps.get(key, []) recent = [t for t in timestamps if t > cutoff] + # Prune expired entries on read + self._timestamps[key] = recent limit = self._config.max_per_pair_per_minute + self._config.burst_allowance if len(recent) >= limit: logger.info( @@ -79,7 +83,8 @@ def check( message=( f"Rate limit exceeded for pair " f"({delegator_id!r}, {delegatee_id!r}): " - f"{len(recent)} delegations in last 60s " + f"{len(recent)} delegations in last " + f"{self._window_seconds:.0f}s " f"(limit {limit})" ), ) @@ -96,9 +101,9 @@ def record( delegator_id: ID of the delegating agent. delegatee_id: ID of the target agent. """ - key = _pair_key(delegator_id, delegatee_id) + key = pair_key(delegator_id, delegatee_id) now = self._clock() - cutoff = now - _WINDOW_SECONDS + cutoff = now - self._window_seconds timestamps = self._timestamps.get(key, []) # Prune expired entries on write self._timestamps[key] = [t for t in timestamps if t > cutoff] + [now] diff --git a/src/ai_company/core/task.py b/src/ai_company/core/task.py index cdc06a4c30..bf549d4430 100644 --- a/src/ai_company/core/task.py +++ b/src/ai_company/core/task.py @@ -60,6 +60,9 @@ class Task(BaseModel): deadline: Optional deadline (ISO 8601 string or ``None``). max_retries: Max reassignment attempts after failure (default 1). status: Current lifecycle status. + parent_task_id: Parent task ID when created via delegation + (``None`` for root tasks). + delegation_chain: Ordered agent IDs of delegators (root first). """ model_config = ConfigDict(frozen=True) @@ -140,9 +143,9 @@ def _validate_deadline_format(self) -> Self: raise ValueError(msg) try: datetime.fromisoformat(self.deadline) - except ValueError: + except ValueError as exc: msg = f"deadline must be a valid ISO 8601 string, got {self.deadline!r}" - raise ValueError(msg) from None + raise ValueError(msg) from exc return self @model_validator(mode="after") diff --git a/src/ai_company/observability/events/delegation.py b/src/ai_company/observability/events/delegation.py index 3963a253fb..64afd4e3e3 100644 --- a/src/ai_company/observability/events/delegation.py +++ b/src/ai_company/observability/events/delegation.py @@ -16,6 +16,7 @@ DELEGATION_LOOP_DEDUP_BLOCKED: Final[str] = "delegation.loop.dedup_blocked" DELEGATION_LOOP_RATE_LIMITED: Final[str] = "delegation.loop.rate_limited" DELEGATION_LOOP_CIRCUIT_OPEN: Final[str] = "delegation.loop.circuit_open" +DELEGATION_LOOP_CIRCUIT_RESET: Final[str] = "delegation.loop.circuit_reset" DELEGATION_LOOP_ESCALATED: Final[str] = "delegation.loop.escalated" # Hierarchy diff --git a/tests/unit/communication/delegation/test_hierarchy.py b/tests/unit/communication/delegation/test_hierarchy.py index c7f7c749a2..886ab48c94 100644 --- a/tests/unit/communication/delegation/test_hierarchy.py +++ b/tests/unit/communication/delegation/test_hierarchy.py @@ -252,6 +252,78 @@ def test_reporting_line_overrides_team_structure(self) -> None: assert resolver.get_supervisor("sr_dev") == "cto" +@pytest.mark.unit +class TestEdgeCases: + def test_lead_in_members_list_not_duplicated(self) -> None: + """Team lead appearing in members should not be registered twice.""" + dept = Department( + name="Engineering", + head="cto", + budget_percent=50.0, + teams=( + Team( + name="backend", + lead="backend_lead", + members=("backend_lead", "dev1"), + ), + ), + ) + company = _make_company(departments=(dept,)) + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("backend_lead") == "cto" + reports = resolver.get_direct_reports("backend_lead") + assert reports.count("dev1") == 1 + assert "backend_lead" not in reports + + def test_multi_team_lead_keeps_first_supervisor(self) -> None: + """Agent leading two teams keeps the first team's dept head.""" + dept = Department( + name="Engineering", + head="cto", + budget_percent=50.0, + teams=( + Team(name="t1", lead="shared_lead", members=("dev1",)), + Team(name="t2", lead="shared_lead", members=("dev2",)), + ), + ) + company = _make_company(departments=(dept,)) + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("shared_lead") == "cto" + assert resolver.get_supervisor("dev1") == "shared_lead" + assert resolver.get_supervisor("dev2") == "shared_lead" + + def test_member_with_prior_supervisor_keeps_first(self) -> None: + """Member already assigned to a supervisor is not re-assigned.""" + dept = Department( + name="Engineering", + head="cto", + budget_percent=50.0, + teams=( + Team(name="t1", lead="lead1", members=("dev1",)), + Team(name="t2", lead="lead2", members=("dev1",)), + ), + ) + company = _make_company(departments=(dept,)) + resolver = HierarchyResolver(company) + # dev1 seen first under lead1 + assert resolver.get_supervisor("dev1") == "lead1" + + def test_redundant_reporting_line_no_duplicate(self) -> None: + """Explicit line matching inferred relationship doesn't duplicate.""" + dept = Department( + name="Engineering", + head="cto", + budget_percent=50.0, + teams=(Team(name="backend", lead="lead1", members=("dev1",)),), + reporting_lines=(ReportingLine(subordinate="dev1", supervisor="lead1"),), + ) + company = _make_company(departments=(dept,)) + resolver = HierarchyResolver(company) + assert resolver.get_supervisor("dev1") == "lead1" + reports = resolver.get_direct_reports("lead1") + assert reports.count("dev1") == 1 + + @pytest.mark.unit class TestCycleDetection: def test_cycle_raises_hierarchy_error(self) -> None: diff --git a/tests/unit/communication/delegation/test_models.py b/tests/unit/communication/delegation/test_models.py index 9219031ed5..47b771f917 100644 --- a/tests/unit/communication/delegation/test_models.py +++ b/tests/unit/communication/delegation/test_models.py @@ -110,10 +110,37 @@ def test_failure_result(self) -> None: assert result.blocked_by == "hierarchy" def test_frozen(self) -> None: - result = DelegationResult(success=True) + task = _make_task() + result = DelegationResult(success=True, delegated_task=task) with pytest.raises(ValidationError): result.success = False # type: ignore[misc] + def test_success_without_task_rejected(self) -> None: + with pytest.raises(ValidationError, match="delegated_task is required"): + DelegationResult(success=True) + + def test_success_with_rejection_reason_rejected(self) -> None: + task = _make_task() + with pytest.raises(ValidationError, match="rejection_reason must be None"): + DelegationResult( + success=True, + delegated_task=task, + rejection_reason="oops", + ) + + def test_success_with_blocked_by_rejected(self) -> None: + task = _make_task() + with pytest.raises(ValidationError, match="blocked_by must be None"): + DelegationResult( + success=True, + delegated_task=task, + blocked_by="guard", + ) + + def test_failure_without_reason_rejected(self) -> None: + with pytest.raises(ValidationError, match="rejection_reason is required"): + DelegationResult(success=False) + @pytest.mark.unit class TestDelegationRecord: diff --git a/tests/unit/communication/loop_prevention/test_dedup.py b/tests/unit/communication/loop_prevention/test_dedup.py index abd67af1e1..5c69561e67 100644 --- a/tests/unit/communication/loop_prevention/test_dedup.py +++ b/tests/unit/communication/loop_prevention/test_dedup.py @@ -64,6 +64,33 @@ def clock() -> float: result = dedup.check("a", "c", "task-1") assert result.passed is True + def test_directional_key_a_to_b_distinct_from_b_to_a(self) -> None: + """Dedup uses directional keys: A->B and B->A are distinct.""" + clock_time = 100.0 + + def clock() -> float: + return clock_time + + dedup = DelegationDeduplicator(window_seconds=60, clock=clock) + dedup.record("a", "b", "task-1") + # Reverse direction should pass (directional) + result = dedup.check("b", "a", "task-1") + assert result.passed is True + + def test_expired_entries_pruned_on_check(self) -> None: + """Expired entries are removed when check detects them.""" + clock_time = 100.0 + + def clock() -> float: + return clock_time + + dedup = DelegationDeduplicator(window_seconds=60, clock=clock) + dedup.record("a", "b", "task-1") + clock_time = 161.0 # expired + dedup.check("a", "b", "task-1") + # Internal record should be pruned + assert ("a", "b", "task-1") not in dedup._records + def test_record_updates_timestamp(self) -> None: clock_time = 100.0 From a9a1ba932f06b936611187f74f0026c6a9b1e37f Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sat, 7 Mar 2026 20:08:17 +0100 Subject: [PATCH 3/4] refactor: address 33 PR review findings from local agents and external reviewers Source fixes: - Add _escalate_loop_detection method with supervisor lookup and logging - Add result event logging (DELEGATION_RESULT_SENT) on successful delegation - Change guard/dedup API from task_title to task_id for correct dedup identity - Change logger.error to logger.exception in sub-task creation error path - Add identity consistency checks (delegator/delegatee ID vs name mismatch) - Add self-delegation model validator on DelegationRequest - Add failure-with-task validator on DelegationResult - Add whitespace-only message rejection on GuardCheckOutcome - Add max_depth <= 0 validation on depth check - Add blank agent ID validation on pair_key utility - Split circuit breaker _get_pair into _get_pair/_get_or_create_pair - Circuit breaker evicts stale entries on cooldown expiry - Rate limiter evicts empty timestamp lists - Dedup adds global _purge_expired sweep on every check/record - Circuit breaker record_delegation is no-op when circuit is open - Constraints appended to sub-task description - Sub-task inherits max_retries from original task Documentation fixes: - Fix DESIGN_SPEC.md: CircuitBreakerState listed in correct file - Add __init__.py entries for delegation/ and loop_prevention/ in DESIGN_SPEC - Update docstrings across hierarchy, authority, depth, circuit breaker, guard Test fixes: - Fix test_ancestry_blocked to use enforce_chain=False (not allow_skip=True) - Rename record_bounce -> record_delegation in circuit breaker tests - Rename test_different_task_title_passes -> test_different_task_id_passes - Update guard tests from task_title to task_id parameter - Add identity mismatch, constraints, self-delegation, failure-with-task tests - Add circuit breaker no-op-when-open test - Add dedup global pruning test - Add delegation event value assertions in test_events.py - Parametrize ancestry failure tests - Update integration test comments for task_id-based dedup Co-Authored-By: Claude Opus 4.6 --- DESIGN_SPEC.md | 6 +- .../communication/delegation/authority.py | 3 +- .../communication/delegation/hierarchy.py | 6 +- .../communication/delegation/models.py | 13 +- .../communication/delegation/service.py | 122 +++++++++++++++--- .../loop_prevention/_pair_key.py | 17 ++- .../loop_prevention/circuit_breaker.py | 31 +++-- .../communication/loop_prevention/dedup.py | 73 ++++++----- .../communication/loop_prevention/depth.py | 10 +- .../communication/loop_prevention/guard.py | 65 ++++++---- .../communication/loop_prevention/models.py | 2 +- .../loop_prevention/rate_limit.py | 18 ++- .../test_delegation_integration.py | 2 +- .../communication/delegation/test_models.py | 18 +++ .../communication/delegation/test_service.py | 60 ++++++++- .../loop_prevention/test_ancestry.py | 30 ++--- .../loop_prevention/test_circuit_breaker.py | 35 +++-- .../loop_prevention/test_dedup.py | 18 ++- .../loop_prevention/test_guard.py | 6 +- tests/unit/observability/test_events.py | 18 +++ 20 files changed, 413 insertions(+), 140 deletions(-) diff --git a/DESIGN_SPEC.md b/DESIGN_SPEC.md index 95c3fb976f..7bab40bb00 100644 --- a/DESIGN_SPEC.md +++ b/DESIGN_SPEC.md @@ -2353,6 +2353,7 @@ ai-company/ │ │ ├── channel.py # Channel model │ │ ├── config.py # Communication config │ │ ├── delegation/ # Hierarchical delegation subsystem +│ │ │ ├── __init__.py # Package exports │ │ │ ├── authority.py # AuthorityValidator + AuthorityCheckResult │ │ │ ├── hierarchy.py # HierarchyResolver (org hierarchy from Company) │ │ │ ├── models.py # DelegationRequest, DelegationResult, DelegationRecord @@ -2362,13 +2363,14 @@ ai-company/ │ │ ├── errors.py # Communication + delegation error hierarchy │ │ ├── handler.py # MessageHandler protocol, FunctionHandler, HandlerRegistration │ │ ├── loop_prevention/ # Delegation loop prevention mechanisms +│ │ │ ├── __init__.py # Package exports │ │ │ ├── _pair_key.py # Canonical agent-pair key utility │ │ │ ├── ancestry.py # Ancestry cycle detection (pure function) -│ │ │ ├── circuit_breaker.py # DelegationCircuitBreaker (per-pair) +│ │ │ ├── circuit_breaker.py # DelegationCircuitBreaker, CircuitBreakerState │ │ │ ├── dedup.py # DelegationDeduplicator (time-windowed) │ │ │ ├── depth.py # Max delegation depth check (pure function) │ │ │ ├── guard.py # DelegationGuard (orchestrates all mechanisms) -│ │ │ ├── models.py # GuardCheckOutcome, CircuitBreakerState +│ │ │ ├── models.py # GuardCheckOutcome │ │ │ └── rate_limit.py # DelegationRateLimiter (per-pair) │ │ ├── message.py # Message model │ │ ├── messenger.py # AgentMessenger per-agent facade diff --git a/src/ai_company/communication/delegation/authority.py b/src/ai_company/communication/delegation/authority.py index 6e711f004d..970ece26c1 100644 --- a/src/ai_company/communication/delegation/authority.py +++ b/src/ai_company/communication/delegation/authority.py @@ -50,7 +50,8 @@ class AuthorityValidator: 1. Hierarchy: delegatee must be a subordinate of delegator (direct or skip-level depending on config). 2. Roles: if ``delegator.authority.can_delegate_to`` is - non-empty, ``delegatee.role`` must be in it. + non-empty, ``delegatee.role`` must be in it; if empty, + all roles are permitted. Args: hierarchy: Resolved org hierarchy. diff --git a/src/ai_company/communication/delegation/hierarchy.py b/src/ai_company/communication/delegation/hierarchy.py index 11f3e926ae..9b8cda78d8 100644 --- a/src/ai_company/communication/delegation/hierarchy.py +++ b/src/ai_company/communication/delegation/hierarchy.py @@ -88,10 +88,10 @@ def __init__(self, company: Company) -> None: @staticmethod def _detect_cycles(supervisor_of: dict[str, str]) -> None: - """Detect cycles in the supervisor graph via single-pass DFS. + """Detect cycles in the supervisor graph via single-pass chain walking. - Uses a visited/in-stack colouring approach for O(n) - complexity instead of restarting traversal per node. + Uses a visited/in-stack approach for O(n) complexity — each + agent is processed at most once. Args: supervisor_of: Mapping from agent to supervisor. diff --git a/src/ai_company/communication/delegation/models.py b/src/ai_company/communication/delegation/models.py index 861855d91e..979d048965 100644 --- a/src/ai_company/communication/delegation/models.py +++ b/src/ai_company/communication/delegation/models.py @@ -37,6 +37,14 @@ class DelegationRequest(BaseModel): description="Extra constraints for the delegatee", ) + @model_validator(mode="after") + def _validate_self_delegation(self) -> Self: + """Reject delegation to self.""" + if self.delegator_id == self.delegatee_id: + msg = "delegator_id and delegatee_id must differ" + raise ValueError(msg) + return self + class DelegationResult(BaseModel): """Outcome of a delegation attempt. @@ -77,7 +85,10 @@ def _validate_success_consistency(self) -> Self: if self.blocked_by is not None: msg = "blocked_by must be None when success is True" raise ValueError(msg) - elif self.rejection_reason is None: + elif self.delegated_task is not None: + msg = "delegated_task must be None when success is False" + raise ValueError(msg) + if not self.success and self.rejection_reason is None: msg = "rejection_reason is required when success is False" raise ValueError(msg) return self diff --git a/src/ai_company/communication/delegation/service.py b/src/ai_company/communication/delegation/service.py index bde05fe8d9..5c1600e3df 100644 --- a/src/ai_company/communication/delegation/service.py +++ b/src/ai_company/communication/delegation/service.py @@ -3,6 +3,8 @@ from datetime import UTC, datetime from uuid import uuid4 +from pydantic import ValidationError + from ai_company.communication.delegation.authority import ( # noqa: TC001 AuthorityValidator, ) @@ -14,6 +16,7 @@ DelegationRequest, DelegationResult, ) +from ai_company.communication.errors import DelegationError from ai_company.communication.loop_prevention.guard import ( # noqa: TC001 DelegationGuard, ) @@ -22,7 +25,9 @@ from ai_company.observability import get_logger from ai_company.observability.events.delegation import ( DELEGATION_CREATED, + DELEGATION_LOOP_ESCALATED, DELEGATION_REQUESTED, + DELEGATION_RESULT_SENT, ) logger = get_logger(__name__) @@ -75,7 +80,25 @@ def delegate( Returns: Result indicating success or rejection with reason. + + Raises: + ValueError: If request IDs do not match identity objects. + DelegationError: If sub-task construction fails. """ + # 0. Identity consistency check + if request.delegator_id != delegator.name: + msg = ( + f"request.delegator_id {request.delegator_id!r} does not " + f"match delegator.name {delegator.name!r}" + ) + raise ValueError(msg) + if request.delegatee_id != delegatee.name: + msg = ( + f"request.delegatee_id {request.delegatee_id!r} does not " + f"match delegatee.name {delegatee.name!r}" + ) + raise ValueError(msg) + logger.info( DELEGATION_REQUESTED, delegator=request.delegator_id, @@ -97,9 +120,10 @@ def delegate( delegation_chain=request.task.delegation_chain, delegator_id=request.delegator_id, delegatee_id=request.delegatee_id, - task_title=request.task.title, + task_id=request.task.id, ) if not guard_outcome.passed: + self._escalate_loop_detection(request, guard_outcome.mechanism) return DelegationResult( success=False, rejection_reason=guard_outcome.message, @@ -113,7 +137,7 @@ def delegate( self._guard.record_delegation( request.delegator_id, request.delegatee_id, - request.task.title, + request.task.id, ) record = DelegationRecord( delegation_id=str(uuid4()), @@ -134,45 +158,105 @@ def delegate( delegated_task_id=sub_task.id, ) - return DelegationResult( + result = DelegationResult( success=True, delegated_task=sub_task, ) + logger.debug( + DELEGATION_RESULT_SENT, + delegator=request.delegator_id, + delegatee=request.delegatee_id, + success=True, + ) + return result def _create_sub_task(self, request: DelegationRequest) -> Task: """Create a new sub-task from a delegation request. The sub-task inherits the original task's properties but gets a new ID, parent reference, extended delegation chain, and - CREATED status. + CREATED status. Constraints and refinement are appended to + the description so the delegatee receives full context. Args: request: The delegation request. Returns: New Task with delegation metadata. + + Raises: + DelegationError: If Task construction fails. """ original = request.task new_chain = (*original.delegation_chain, request.delegator_id) description = original.description if request.refinement: - description = ( - f"{original.description}\n\nDelegation context: {request.refinement}" + description = f"{description}\n\nDelegation context: {request.refinement}" + if request.constraints: + constraints_text = "\n".join(f"- {c}" for c in request.constraints) + description = f"{description}\n\nConstraints:\n{constraints_text}" + + try: + return Task( + id=f"del-{uuid4().hex[:12]}", + title=original.title, + description=description, + type=original.type, + priority=original.priority, + project=original.project, + created_by=request.delegator_id, + parent_task_id=original.id, + delegation_chain=new_chain, + estimated_complexity=original.estimated_complexity, + budget_limit=original.budget_limit, + deadline=original.deadline, + max_retries=original.max_retries, ) + except ValidationError as exc: + logger.exception( + "delegation.sub_task_creation_failed", + delegator=request.delegator_id, + delegatee=request.delegatee_id, + original_task_id=original.id, + error=str(exc), + ) + msg = ( + f"Failed to create sub-task for delegation " + f"from {request.delegator_id!r} to " + f"{request.delegatee_id!r}" + ) + raise DelegationError( + msg, + context={ + "delegator_id": request.delegator_id, + "delegatee_id": request.delegatee_id, + "original_task_id": original.id, + }, + ) from exc + + def _escalate_loop_detection( + self, + request: DelegationRequest, + mechanism: str, + ) -> None: + """Log escalation when a loop prevention mechanism blocks delegation. - return Task( - id=f"del-{uuid4().hex[:12]}", - title=original.title, - description=description, - type=original.type, - priority=original.priority, - project=original.project, - created_by=request.delegator_id, - parent_task_id=original.id, - delegation_chain=new_chain, - estimated_complexity=original.estimated_complexity, - budget_limit=original.budget_limit, - deadline=original.deadline, + Looks up the delegator's supervisor and logs the event so that + the supervisor can be notified (notification delivery is an + async concern handled elsewhere). + + Args: + request: The blocked delegation request. + mechanism: Name of the mechanism that blocked. + """ + supervisor = self._hierarchy.get_supervisor(request.delegator_id) + logger.warning( + DELEGATION_LOOP_ESCALATED, + delegator=request.delegator_id, + delegatee=request.delegatee_id, + task_id=request.task.id, + mechanism=mechanism, + supervisor=supervisor, ) def get_audit_trail(self) -> tuple[DelegationRecord, ...]: diff --git a/src/ai_company/communication/loop_prevention/_pair_key.py b/src/ai_company/communication/loop_prevention/_pair_key.py index 7f2244135a..6828f788bf 100644 --- a/src/ai_company/communication/loop_prevention/_pair_key.py +++ b/src/ai_company/communication/loop_prevention/_pair_key.py @@ -1,8 +1,8 @@ -"""Canonical agent-pair key utility. +"""Canonical agent-pair key utility for undirected pair tracking. -Both ``DelegationCircuitBreaker`` and ``DelegationRateLimiter`` track -state per *undirected* agent pair. This helper normalises the key so -that ``(a, b)`` and ``(b, a)`` map to the same entry. +Normalises keys so that ``(a, b)`` and ``(b, a)`` map to the same +entry. Used by stateful loop prevention mechanisms that track +per-pair state without direction sensitivity. """ @@ -18,5 +18,14 @@ def pair_key(a: str, b: str) -> tuple[str, str]: Returns: Lexicographically sorted ``(min, max)`` tuple. + + Raises: + ValueError: If either agent ID is blank. """ + if not a or not a.strip(): + msg = f"pair_key received blank first agent ID: {a!r}" + raise ValueError(msg) + if not b or not b.strip(): + msg = f"pair_key received blank second agent ID: {b!r}" + raise ValueError(msg) return (min(a, b), max(a, b)) diff --git a/src/ai_company/communication/loop_prevention/circuit_breaker.py b/src/ai_company/communication/loop_prevention/circuit_breaker.py index 99ef49ec58..cd89db1890 100644 --- a/src/ai_company/communication/loop_prevention/circuit_breaker.py +++ b/src/ai_company/communication/loop_prevention/circuit_breaker.py @@ -73,6 +73,14 @@ def _get_pair( self, delegator_id: str, delegatee_id: str, + ) -> _PairState | None: + key = pair_key(delegator_id, delegatee_id) + return self._pairs.get(key) + + def _get_or_create_pair( + self, + delegator_id: str, + delegatee_id: str, ) -> _PairState: key = pair_key(delegator_id, delegatee_id) return self._pairs.setdefault(key, _PairState()) @@ -95,19 +103,21 @@ def get_state( Current state of the circuit breaker. """ pair = self._get_pair(delegator_id, delegatee_id) + if pair is None: + return CircuitBreakerState.CLOSED if pair.opened_at is not None: elapsed = self._clock() - pair.opened_at if elapsed < self._config.cooldown_seconds: return CircuitBreakerState.OPEN - # Cooldown expired: reset + # Cooldown expired: evict the stale entry + key = pair_key(delegator_id, delegatee_id) + del self._pairs[key] logger.info( DELEGATION_LOOP_CIRCUIT_RESET, delegator=delegator_id, delegatee=delegatee_id, cooldown_seconds=self._config.cooldown_seconds, ) - pair.bounce_count = 0 - pair.opened_at = None return CircuitBreakerState.CLOSED def check( @@ -142,17 +152,18 @@ def check( ) return GuardCheckOutcome(passed=True, mechanism=_MECHANISM) - def record_bounce( + def record_delegation( self, delegator_id: str, delegatee_id: str, ) -> None: - """Record a delegation bounce for the pair. + """Record a delegation event for the pair. - If the bounce count reaches the threshold, opens the circuit. - If the circuit was previously open and the cooldown has expired, - the bounce count is reset before recording. If the circuit is - already open (cooldown not yet expired), this call is a no-op. + Each delegation between a pair increments the bounce counter. + Back-and-forth patterns trip the breaker fastest because the + key is direction-agnostic. If the count reaches the threshold, + the circuit opens. If the circuit is already open (cooldown not + yet expired), this call is a no-op. Args: delegator_id: First agent ID. @@ -161,7 +172,7 @@ def record_bounce( state = self.get_state(delegator_id, delegatee_id) if state is CircuitBreakerState.OPEN: return - pair = self._get_pair(delegator_id, delegatee_id) + pair = self._get_or_create_pair(delegator_id, delegatee_id) pair.bounce_count += 1 if pair.bounce_count >= self._config.bounce_threshold: pair.opened_at = self._clock() diff --git a/src/ai_company/communication/loop_prevention/dedup.py b/src/ai_company/communication/loop_prevention/dedup.py index 4002796879..4f68db3ed5 100644 --- a/src/ai_company/communication/loop_prevention/dedup.py +++ b/src/ai_company/communication/loop_prevention/dedup.py @@ -15,7 +15,16 @@ class DelegationDeduplicator: - """Rejects duplicate (delegator, delegatee, task_title) within a window. + """Rejects identical delegations within a time window. + + Identity is determined by the directional tuple + ``(delegator_id, delegatee_id, task_id)`` — the unique task ID is + used instead of the title so that different tasks with the same + title are not falsely blocked, and refined re-delegations of the + same task are correctly deduplicated. + + Expired entries are pruned globally on every ``check`` and + ``record`` call. Args: window_seconds: Duration of the dedup window. @@ -34,63 +43,69 @@ def __init__( self._clock = clock self._records: dict[tuple[str, str, str], float] = {} + def _purge_expired(self) -> None: + """Remove all globally expired entries.""" + now = self._clock() + cutoff = now - self._window_seconds + expired = [k for k, ts in self._records.items() if ts <= cutoff] + for k in expired: + del self._records[k] + def check( self, delegator_id: str, delegatee_id: str, - task_title: str, + task_id: str, ) -> GuardCheckOutcome: """Check for duplicate delegation within the window. Args: delegator_id: ID of the delegating agent. delegatee_id: ID of the target agent. - task_title: Title of the task being delegated. + task_id: Unique ID of the task being delegated. Returns: Outcome with passed=False if a duplicate exists. """ + self._purge_expired() # Directional key: A->B and B->A are distinct delegations - key = (delegator_id, delegatee_id, task_title) + key = (delegator_id, delegatee_id, task_id) recorded_at = self._records.get(key) if recorded_at is not None: elapsed = self._clock() - recorded_at - if elapsed >= self._window_seconds: - # Expired: remove stale entry - del self._records[key] - else: - logger.info( - DELEGATION_LOOP_DEDUP_BLOCKED, - delegator=delegator_id, - delegatee=delegatee_id, - task_title=task_title, - elapsed=elapsed, - window=self._window_seconds, - ) - return GuardCheckOutcome( - passed=False, - mechanism=_MECHANISM, - message=( - f"Duplicate delegation " - f"({delegator_id!r} -> {delegatee_id!r}, " - f"{task_title!r}) within " - f"{self._window_seconds}s window" - ), - ) + logger.info( + DELEGATION_LOOP_DEDUP_BLOCKED, + delegator=delegator_id, + delegatee=delegatee_id, + task_id=task_id, + elapsed=elapsed, + window=self._window_seconds, + ) + return GuardCheckOutcome( + passed=False, + mechanism=_MECHANISM, + message=( + f"Duplicate delegation " + f"({delegator_id!r} -> {delegatee_id!r}, " + f"{task_id!r}) within " + f"{self._window_seconds}s window" + ), + ) return GuardCheckOutcome(passed=True, mechanism=_MECHANISM) def record( self, delegator_id: str, delegatee_id: str, - task_title: str, + task_id: str, ) -> None: """Record a delegation for future dedup checks. Args: delegator_id: ID of the delegating agent. delegatee_id: ID of the target agent. - task_title: Title of the task being delegated. + task_id: Unique ID of the task being delegated. """ - key = (delegator_id, delegatee_id, task_title) + self._purge_expired() + key = (delegator_id, delegatee_id, task_id) self._records[key] = self._clock() diff --git a/src/ai_company/communication/loop_prevention/depth.py b/src/ai_company/communication/loop_prevention/depth.py index 6c262df63d..ea37d99acc 100644 --- a/src/ai_company/communication/loop_prevention/depth.py +++ b/src/ai_company/communication/loop_prevention/depth.py @@ -15,15 +15,21 @@ def check_delegation_depth( delegation_chain: tuple[str, ...], max_depth: int, ) -> GuardCheckOutcome: - """Check whether the delegation chain exceeds maximum depth. + """Check whether the delegation chain has reached or exceeded max depth. Args: delegation_chain: Current chain of delegator agent IDs. - max_depth: Maximum allowed chain length. + max_depth: Maximum allowed chain length (must be positive). Returns: Outcome with passed=True if within limit. + + Raises: + ValueError: If ``max_depth`` is not positive. """ + if max_depth <= 0: + msg = f"max_depth must be greater than 0, got {max_depth}" + raise ValueError(msg) if len(delegation_chain) >= max_depth: logger.info( DELEGATION_LOOP_DEPTH_EXCEEDED, diff --git a/src/ai_company/communication/loop_prevention/guard.py b/src/ai_company/communication/loop_prevention/guard.py index a7110b1c5a..ea9ec17005 100644 --- a/src/ai_company/communication/loop_prevention/guard.py +++ b/src/ai_company/communication/loop_prevention/guard.py @@ -57,7 +57,7 @@ def check( delegation_chain: tuple[str, ...], delegator_id: str, delegatee_id: str, - task_title: str, + task_id: str, ) -> GuardCheckOutcome: """Run all loop prevention checks. @@ -68,33 +68,39 @@ def check( delegation_chain: Current delegation ancestry. delegator_id: ID of the delegating agent. delegatee_id: ID of the proposed delegatee. - task_title: Title of the task being delegated. + task_id: Unique ID of the task being delegated. Returns: First failing outcome or an all-passed success. """ - # Pure (stateless) checks first - for outcome in ( - check_ancestry(delegation_chain, delegatee_id), - check_delegation_depth( - delegation_chain, - self._config.max_delegation_depth, - ), - ): - if not outcome.passed: - return self._log_and_return(outcome, delegator_id, delegatee_id) + # Pure (stateless) checks first — sequential to short-circuit + outcome = check_ancestry(delegation_chain, delegatee_id) + if not outcome.passed: + return self._log_and_return(outcome, delegator_id, delegatee_id) + + outcome = check_delegation_depth( + delegation_chain, + self._config.max_delegation_depth, + ) + if not outcome.passed: + return self._log_and_return(outcome, delegator_id, delegatee_id) + # Stateful checks — only run if pure checks passed - for outcome in ( - self._deduplicator.check( - delegator_id, - delegatee_id, - task_title, - ), - self._rate_limiter.check(delegator_id, delegatee_id), - self._circuit_breaker.check(delegator_id, delegatee_id), - ): - if not outcome.passed: - return self._log_and_return(outcome, delegator_id, delegatee_id) + outcome = self._deduplicator.check( + delegator_id, + delegatee_id, + task_id, + ) + if not outcome.passed: + return self._log_and_return(outcome, delegator_id, delegatee_id) + + outcome = self._rate_limiter.check(delegator_id, delegatee_id) + if not outcome.passed: + return self._log_and_return(outcome, delegator_id, delegatee_id) + + outcome = self._circuit_breaker.check(delegator_id, delegatee_id) + if not outcome.passed: + return self._log_and_return(outcome, delegator_id, delegatee_id) return GuardCheckOutcome( passed=True, mechanism=_SUCCESS_MECHANISM, @@ -120,15 +126,20 @@ def record_delegation( self, delegator_id: str, delegatee_id: str, - task_title: str, + task_id: str, ) -> None: """Record a successful delegation in all stateful mechanisms. + Each delegation between a pair contributes to the circuit breaker + bounce count. Back-and-forth patterns (A→B then B→A) both + increment the same counter because the pair key is direction- + agnostic, so repeated ping-pong will trip the breaker fastest. + Args: delegator_id: ID of the delegating agent. delegatee_id: ID of the target agent. - task_title: Title of the delegated task. + task_id: Unique ID of the delegated task. """ - self._deduplicator.record(delegator_id, delegatee_id, task_title) + self._deduplicator.record(delegator_id, delegatee_id, task_id) self._rate_limiter.record(delegator_id, delegatee_id) - self._circuit_breaker.record_bounce(delegator_id, delegatee_id) + self._circuit_breaker.record_delegation(delegator_id, delegatee_id) diff --git a/src/ai_company/communication/loop_prevention/models.py b/src/ai_company/communication/loop_prevention/models.py index c3c4c1d4bd..842ca30f3f 100644 --- a/src/ai_company/communication/loop_prevention/models.py +++ b/src/ai_company/communication/loop_prevention/models.py @@ -33,7 +33,7 @@ def _validate_passed_message(self) -> Self: if self.passed and self.message: msg = "message must be empty when passed is True" raise ValueError(msg) - if not self.passed and not self.message: + if not self.passed and not self.message.strip(): msg = "message is required when passed is False" raise ValueError(msg) return self diff --git a/src/ai_company/communication/loop_prevention/rate_limit.py b/src/ai_company/communication/loop_prevention/rate_limit.py index 61183e4655..dd5f924b84 100644 --- a/src/ai_company/communication/loop_prevention/rate_limit.py +++ b/src/ai_company/communication/loop_prevention/rate_limit.py @@ -21,8 +21,9 @@ class DelegationRateLimiter: """Per-pair rate limit with burst allowance. The key is the sorted (a, b) agent pair. Counts delegations within - the sliding window. If the count exceeds - ``max_per_pair_per_minute + burst_allowance``, the check fails. + the sliding window. The effective limit per window is + ``max_per_pair_per_minute + burst_allowance``, giving additive + headroom above the base rate. Args: config: Rate limit configuration. @@ -66,8 +67,11 @@ def check( cutoff = now - self._window_seconds timestamps = self._timestamps.get(key, []) recent = [t for t in timestamps if t > cutoff] - # Prune expired entries on read - self._timestamps[key] = recent + # Prune expired entries on read; evict empty keys + if recent: + self._timestamps[key] = recent + else: + self._timestamps.pop(key, None) limit = self._config.max_per_pair_per_minute + self._config.burst_allowance if len(recent) >= limit: logger.info( @@ -105,5 +109,7 @@ def record( now = self._clock() cutoff = now - self._window_seconds timestamps = self._timestamps.get(key, []) - # Prune expired entries on write - self._timestamps[key] = [t for t in timestamps if t > cutoff] + [now] + # Prune expired entries on write; add new timestamp + recent = [t for t in timestamps if t > cutoff] + recent.append(now) + self._timestamps[key] = recent diff --git a/tests/integration/communication/test_delegation_integration.py b/tests/integration/communication/test_delegation_integration.py index 7a426ec935..3052d8e01d 100644 --- a/tests/integration/communication/test_delegation_integration.py +++ b/tests/integration/communication/test_delegation_integration.py @@ -158,7 +158,7 @@ def test_ceo_to_cto_to_dev(self) -> None: assert sub1.status is TaskStatus.CREATED assert "Focus on backend API" in sub1.description - # CTO → Dev (different title to avoid dedup) + # CTO → Dev (different task ID avoids dedup naturally) sub1_retitled = Task( id=sub1.id, title="Build auth API endpoints", diff --git a/tests/unit/communication/delegation/test_models.py b/tests/unit/communication/delegation/test_models.py index 47b771f917..82cab90a6c 100644 --- a/tests/unit/communication/delegation/test_models.py +++ b/tests/unit/communication/delegation/test_models.py @@ -84,6 +84,15 @@ def test_blank_delegatee_rejected(self) -> None: task=task, ) + def test_self_delegation_rejected(self) -> None: + task = _make_task() + with pytest.raises(ValidationError, match="must differ"): + DelegationRequest( + delegator_id="cto", + delegatee_id="cto", + task=task, + ) + @pytest.mark.unit class TestDelegationResult: @@ -141,6 +150,15 @@ def test_failure_without_reason_rejected(self) -> None: with pytest.raises(ValidationError, match="rejection_reason is required"): DelegationResult(success=False) + def test_failure_with_task_rejected(self) -> None: + task = _make_task() + with pytest.raises(ValidationError, match="delegated_task must be None"): + DelegationResult( + success=False, + delegated_task=task, + rejection_reason="blocked", + ) + @pytest.mark.unit class TestDelegationRecord: diff --git a/tests/unit/communication/delegation/test_service.py b/tests/unit/communication/delegation/test_service.py index f763de20c5..b424404395 100644 --- a/tests/unit/communication/delegation/test_service.py +++ b/tests/unit/communication/delegation/test_service.py @@ -232,7 +232,7 @@ def test_role_permission_denied(self) -> None: @pytest.mark.unit class TestDelegationServiceLoopPrevention: def test_ancestry_blocked(self) -> None: - service, _ = _build_service(allow_skip=True) + service, _ = _build_service(enforce_chain=False) # First delegation: ceo -> cto task1 = _make_task() ceo = _make_agent("ceo", "ceo") @@ -257,7 +257,7 @@ def test_ancestry_blocked(self) -> None: r2 = service.delegate(req2, cto, dev) assert r2.success is True - # Third: dev tries to delegate back to ceo → blocked + # Third: dev tries to delegate back to ceo → blocked by ancestry sub2 = r2.delegated_task assert sub2 is not None req3 = DelegationRequest( @@ -265,10 +265,9 @@ def test_ancestry_blocked(self) -> None: delegatee_id="ceo", task=sub2, ) - # Chain of command won't matter here — ancestry blocks first r3 = service.delegate(req3, dev, ceo) - # Either authority or ancestry blocks it assert r3.success is False + assert r3.blocked_by == "ancestry" def test_depth_exceeded(self) -> None: service, _ = _build_service(max_depth=1) @@ -297,7 +296,7 @@ def test_dedup_blocked(self) -> None: # First succeeds r1 = service.delegate(request, delegator, delegatee) assert r1.success is True - # Second is dedup blocked (same delegator, delegatee, title) + # Second is dedup blocked (same delegator, delegatee, task ID) r2 = service.delegate(request, delegator, delegatee) assert r2.success is False assert r2.blocked_by == "dedup" @@ -325,7 +324,7 @@ def test_multi_level_delegation_chain(self) -> None: assert sub1 is not None assert sub1.delegation_chain == ("ceo",) - # CTO → Dev (using sub-task, different title to avoid dedup) + # CTO → Dev (using sub-task, different task ID avoids dedup) sub1_new_title = Task( id=sub1.id, title="Build feature X - backend", @@ -387,6 +386,55 @@ def test_audit_trail_multi_hop(self) -> None: assert trail[1].delegator_id == "cto" +@pytest.mark.unit +class TestDelegationServiceIdentityValidation: + def test_delegator_id_mismatch_raises(self) -> None: + service, _ = _build_service() + task = _make_task() + delegatee = _make_agent("cto", "cto") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + wrong_delegator = _make_agent("imposter", "imposter") + with pytest.raises(ValueError, match="delegator_id"): + service.delegate(request, wrong_delegator, delegatee) + + def test_delegatee_id_mismatch_raises(self) -> None: + service, _ = _build_service() + task = _make_task() + delegator = _make_agent("ceo", "ceo") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + ) + wrong_delegatee = _make_agent("imposter", "imposter") + with pytest.raises(ValueError, match="delegatee_id"): + service.delegate(request, delegator, wrong_delegatee) + + +@pytest.mark.unit +class TestDelegationServiceConstraints: + def test_constraints_appended_to_description(self) -> None: + service, _ = _build_service() + task = _make_task() + delegator = _make_agent("ceo", "ceo") + delegatee = _make_agent("cto", "cto") + request = DelegationRequest( + delegator_id="ceo", + delegatee_id="cto", + task=task, + constraints=("no-external-deps", "max-2-files"), + ) + result = service.delegate(request, delegator, delegatee) + sub = result.delegated_task + assert sub is not None + assert "- no-external-deps" in sub.description + assert "- max-2-files" in sub.description + + @pytest.mark.unit class TestDelegationServiceHelpers: def test_get_supervisor_of(self) -> None: diff --git a/tests/unit/communication/loop_prevention/test_ancestry.py b/tests/unit/communication/loop_prevention/test_ancestry.py index 23277b9f20..b494ba3121 100644 --- a/tests/unit/communication/loop_prevention/test_ancestry.py +++ b/tests/unit/communication/loop_prevention/test_ancestry.py @@ -20,23 +20,23 @@ def test_delegatee_not_in_chain_passes(self) -> None: result = check_ancestry(("a", "b", "c"), "d") assert result.passed is True - def test_delegatee_in_chain_fails(self) -> None: - result = check_ancestry(("a", "b", "c"), "b") + @pytest.mark.parametrize( + ("chain", "delegatee"), + [ + (("a", "b", "c"), "b"), + (("root", "mid"), "root"), + (("a", "b"), "b"), + (("x",), "x"), + ], + ids=["mid-chain", "root", "last-in-chain", "single-element"], + ) + def test_delegatee_in_chain_fails( + self, chain: tuple[str, ...], delegatee: str + ) -> None: + result = check_ancestry(chain, delegatee) assert result.passed is False assert result.mechanism == "ancestry" - assert "'b'" in result.message - - def test_delegatee_is_root_fails(self) -> None: - result = check_ancestry(("root", "mid"), "root") - assert result.passed is False - - def test_delegatee_is_last_in_chain_fails(self) -> None: - result = check_ancestry(("a", "b"), "b") - assert result.passed is False - - def test_single_element_chain_match_fails(self) -> None: - result = check_ancestry(("x",), "x") - assert result.passed is False + assert f"'{delegatee}'" in result.message def test_single_element_chain_no_match_passes(self) -> None: result = check_ancestry(("x",), "y") diff --git a/tests/unit/communication/loop_prevention/test_circuit_breaker.py b/tests/unit/communication/loop_prevention/test_circuit_breaker.py index 2e976bcc95..62d2b0ff3d 100644 --- a/tests/unit/communication/loop_prevention/test_circuit_breaker.py +++ b/tests/unit/communication/loop_prevention/test_circuit_breaker.py @@ -34,7 +34,7 @@ def clock() -> float: cb = DelegationCircuitBreaker(config, clock=clock) for _ in range(3): - cb.record_bounce("a", "b") + cb.record_delegation("a", "b") assert cb.get_state("a", "b") is CircuitBreakerState.OPEN def test_check_fails_when_open(self) -> None: @@ -46,7 +46,7 @@ def clock() -> float: cb = DelegationCircuitBreaker(config, clock=clock) for _ in range(3): - cb.record_bounce("a", "b") + cb.record_delegation("a", "b") result = cb.check("a", "b") assert result.passed is False assert result.mechanism == "circuit_breaker" @@ -60,7 +60,7 @@ def clock() -> float: cb = DelegationCircuitBreaker(config, clock=clock) for _ in range(3): - cb.record_bounce("a", "b") + cb.record_delegation("a", "b") assert cb.get_state("a", "b") is CircuitBreakerState.OPEN clock_time = 401.0 # 301s later @@ -77,21 +77,38 @@ def clock() -> float: return clock_time cb = DelegationCircuitBreaker(config, clock=clock) - cb.record_bounce("b", "a") - cb.record_bounce("a", "b") + cb.record_delegation("b", "a") + cb.record_delegation("a", "b") assert cb.get_state("a", "b") is CircuitBreakerState.OPEN def test_below_threshold_stays_closed(self) -> None: config = CircuitBreakerConfig(bounce_threshold=3, cooldown_seconds=300) cb = DelegationCircuitBreaker(config) - cb.record_bounce("a", "b") - cb.record_bounce("a", "b") + cb.record_delegation("a", "b") + cb.record_delegation("a", "b") assert cb.get_state("a", "b") is CircuitBreakerState.CLOSED def test_different_pair_independent(self) -> None: config = CircuitBreakerConfig(bounce_threshold=2, cooldown_seconds=60) cb = DelegationCircuitBreaker(config) - cb.record_bounce("a", "b") - cb.record_bounce("a", "b") + cb.record_delegation("a", "b") + cb.record_delegation("a", "b") assert cb.get_state("a", "b") is CircuitBreakerState.OPEN assert cb.get_state("a", "c") is CircuitBreakerState.CLOSED + + def test_record_delegation_noop_when_open(self) -> None: + """Recording while circuit is open does not affect the state.""" + config = CircuitBreakerConfig(bounce_threshold=2, cooldown_seconds=300) + clock_time = 100.0 + + def clock() -> float: + return clock_time + + cb = DelegationCircuitBreaker(config, clock=clock) + cb.record_delegation("a", "b") + cb.record_delegation("a", "b") + assert cb.get_state("a", "b") is CircuitBreakerState.OPEN + # Recording while open is a no-op + cb.record_delegation("a", "b") + # Should still be open, cooldown hasn't changed + assert cb.get_state("a", "b") is CircuitBreakerState.OPEN diff --git a/tests/unit/communication/loop_prevention/test_dedup.py b/tests/unit/communication/loop_prevention/test_dedup.py index 5c69561e67..cbd3264c1a 100644 --- a/tests/unit/communication/loop_prevention/test_dedup.py +++ b/tests/unit/communication/loop_prevention/test_dedup.py @@ -42,7 +42,7 @@ def clock() -> float: result = dedup.check("a", "b", "task-1") assert result.passed is True - def test_different_task_title_passes(self) -> None: + def test_different_task_id_passes(self) -> None: clock_time = 100.0 def clock() -> float: @@ -104,3 +104,19 @@ def clock() -> float: clock_time = 200.0 # 100s after first, 50s after second result = dedup.check("a", "b", "task-1") assert result.passed is False # still within window of 2nd + + def test_global_purge_removes_all_expired(self) -> None: + """Multiple expired entries are pruned in a single sweep.""" + clock_time = 100.0 + + def clock() -> float: + return clock_time + + dedup = DelegationDeduplicator(window_seconds=60, clock=clock) + dedup.record("a", "b", "task-1") + dedup.record("c", "d", "task-2") + dedup.record("e", "f", "task-3") + clock_time = 161.0 # all expired + # Trigger purge via check + dedup.check("x", "y", "task-new") + assert len(dedup._records) == 0 diff --git a/tests/unit/communication/loop_prevention/test_guard.py b/tests/unit/communication/loop_prevention/test_guard.py index f313647da0..632ba9c3b5 100644 --- a/tests/unit/communication/loop_prevention/test_guard.py +++ b/tests/unit/communication/loop_prevention/test_guard.py @@ -36,7 +36,7 @@ def test_all_checks_pass(self) -> None: delegation_chain=("ceo",), delegator_id="ceo", delegatee_id="cto", - task_title="Build feature X", + task_id="Build feature X", ) assert result.passed is True assert result.mechanism == "all_passed" @@ -47,7 +47,7 @@ def test_ancestry_blocked(self) -> None: delegation_chain=("ceo", "cto"), delegator_id="cto", delegatee_id="ceo", - task_title="Build feature X", + task_id="Build feature X", ) assert result.passed is False assert result.mechanism == "ancestry" @@ -58,7 +58,7 @@ def test_depth_exceeded(self) -> None: delegation_chain=("a", "b"), delegator_id="b", delegatee_id="c", - task_title="Task", + task_id="Task", ) assert result.passed is False assert result.mechanism == "max_depth" diff --git a/tests/unit/observability/test_events.py b/tests/unit/observability/test_events.py index 27d9be845c..fa04bb4a59 100644 --- a/tests/unit/observability/test_events.py +++ b/tests/unit/observability/test_events.py @@ -21,6 +21,15 @@ CONFIG_PARSE_FAILED, CONFIG_VALIDATION_FAILED, ) +from ai_company.observability.events.delegation import ( + DELEGATION_CREATED, + DELEGATION_HIERARCHY_BUILT, + DELEGATION_HIERARCHY_CYCLE, + DELEGATION_LOOP_BLOCKED, + DELEGATION_LOOP_ESCALATED, + DELEGATION_REQUESTED, + DELEGATION_RESULT_SENT, +) from ai_company.observability.events.execution import EXECUTION_TASK_CREATED from ai_company.observability.events.git import ( GIT_CLONE_URL_REJECTED, @@ -183,5 +192,14 @@ def test_communication_events_exist(self) -> None: assert COMM_HANDLER_DEREGISTER_MISS == "communication.handler.deregister_miss" assert COMM_DISPATCH_NO_DISPATCHER == "communication.dispatch.no_dispatcher" + def test_delegation_events_exist(self) -> None: + assert DELEGATION_REQUESTED == "delegation.requested" + assert DELEGATION_CREATED == "delegation.created" + assert DELEGATION_RESULT_SENT == "delegation.result_sent" + assert DELEGATION_LOOP_BLOCKED == "delegation.loop.blocked" + assert DELEGATION_LOOP_ESCALATED == "delegation.loop.escalated" + assert DELEGATION_HIERARCHY_BUILT == "delegation.hierarchy.built" + assert DELEGATION_HIERARCHY_CYCLE == "delegation.hierarchy.cycle" + def test_tool_events_exist(self) -> None: assert TOOL_INVOKE_START == "tool.invoke.start" From 1d8d0103e53deea06f19c330f4321c38c2f9a2e7 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sat, 7 Mar 2026 20:24:27 +0100 Subject: [PATCH 4/4] refactor: address 7 post-push review findings from CodeRabbit and Greptile - Fix hierarchy self-cycle when dept.head == team.lead (CodeRabbit) - Reject whitespace-only denial reasons in AuthorityCheckResult (CodeRabbit) - Reject whitespace-only rejection_reason in DelegationResult (CodeRabbit) - Promote DELEGATION_AUTHORIZED to logger.info (CodeRabbit) - Add DELEGATION_SUB_TASK_FAILED event constant, replace bare string (CodeRabbit) - Copy reviewers/dependencies/artifacts_expected/acceptance_criteria to sub-task (Greptile) - Extract _validate_identity and _record_delegation helpers from delegate() (CodeRabbit) - Add test for dept-head-as-team-lead self-cycle prevention Co-Authored-By: Claude Opus 4.6 --- .../communication/delegation/authority.py | 4 +- .../communication/delegation/hierarchy.py | 2 +- .../communication/delegation/models.py | 4 +- .../communication/delegation/service.py | 73 +++++++++++++------ .../observability/events/delegation.py | 1 + .../delegation/test_hierarchy.py | 14 ++++ 6 files changed, 72 insertions(+), 26 deletions(-) diff --git a/src/ai_company/communication/delegation/authority.py b/src/ai_company/communication/delegation/authority.py index 970ece26c1..37594977cd 100644 --- a/src/ai_company/communication/delegation/authority.py +++ b/src/ai_company/communication/delegation/authority.py @@ -37,7 +37,7 @@ def _validate_allowed_reason(self) -> Self: if self.allowed and self.reason: msg = "reason must be empty when allowed is True" raise ValueError(msg) - if not self.allowed and not self.reason: + if not self.allowed and not self.reason.strip(): msg = "reason is required when allowed is False" raise ValueError(msg) return self @@ -91,7 +91,7 @@ def validate( if not result.allowed: return result - logger.debug( + logger.info( DELEGATION_AUTHORIZED, delegator=delegator.name, delegatee=delegatee.name, diff --git a/src/ai_company/communication/delegation/hierarchy.py b/src/ai_company/communication/delegation/hierarchy.py index 9b8cda78d8..529d124622 100644 --- a/src/ai_company/communication/delegation/hierarchy.py +++ b/src/ai_company/communication/delegation/hierarchy.py @@ -41,7 +41,7 @@ def __init__(self, company: Company) -> None: for dept in company.departments: for team in dept.teams: # Team lead -> department head (lowest priority) - if team.lead not in supervisor_of: + if team.lead != dept.head and team.lead not in supervisor_of: supervisor_of[team.lead] = dept.head reports_of.setdefault(dept.head, []).append(team.lead) diff --git a/src/ai_company/communication/delegation/models.py b/src/ai_company/communication/delegation/models.py index 979d048965..3a1022a58b 100644 --- a/src/ai_company/communication/delegation/models.py +++ b/src/ai_company/communication/delegation/models.py @@ -88,7 +88,9 @@ def _validate_success_consistency(self) -> Self: elif self.delegated_task is not None: msg = "delegated_task must be None when success is False" raise ValueError(msg) - if not self.success and self.rejection_reason is None: + if not self.success and ( + self.rejection_reason is None or not self.rejection_reason.strip() + ): msg = "rejection_reason is required when success is False" raise ValueError(msg) return self diff --git a/src/ai_company/communication/delegation/service.py b/src/ai_company/communication/delegation/service.py index 5c1600e3df..bb92ec0025 100644 --- a/src/ai_company/communication/delegation/service.py +++ b/src/ai_company/communication/delegation/service.py @@ -28,6 +28,7 @@ DELEGATION_LOOP_ESCALATED, DELEGATION_REQUESTED, DELEGATION_RESULT_SENT, + DELEGATION_SUB_TASK_FAILED, ) logger = get_logger(__name__) @@ -85,19 +86,7 @@ def delegate( ValueError: If request IDs do not match identity objects. DelegationError: If sub-task construction fails. """ - # 0. Identity consistency check - if request.delegator_id != delegator.name: - msg = ( - f"request.delegator_id {request.delegator_id!r} does not " - f"match delegator.name {delegator.name!r}" - ) - raise ValueError(msg) - if request.delegatee_id != delegatee.name: - msg = ( - f"request.delegatee_id {request.delegatee_id!r} does not " - f"match delegatee.name {delegatee.name!r}" - ) - raise ValueError(msg) + self._validate_identity(request, delegator, delegatee) logger.info( DELEGATION_REQUESTED, @@ -130,10 +119,52 @@ def delegate( blocked_by=guard_outcome.mechanism, ) - # 3. Create sub-task + # 3. Create sub-task and record sub_task = self._create_sub_task(request) + self._record_delegation(request, sub_task) + + return DelegationResult(success=True, delegated_task=sub_task) + + @staticmethod + def _validate_identity( + request: DelegationRequest, + delegator: AgentIdentity, + delegatee: AgentIdentity, + ) -> None: + """Verify request IDs match the identity objects. + + Args: + request: The delegation request. + delegator: Identity of the delegating agent. + delegatee: Identity of the target agent. + + Raises: + ValueError: If IDs do not match. + """ + if request.delegator_id != delegator.name: + msg = ( + f"request.delegator_id {request.delegator_id!r} does not " + f"match delegator.name {delegator.name!r}" + ) + raise ValueError(msg) + if request.delegatee_id != delegatee.name: + msg = ( + f"request.delegatee_id {request.delegatee_id!r} does not " + f"match delegatee.name {delegatee.name!r}" + ) + raise ValueError(msg) + + def _record_delegation( + self, + request: DelegationRequest, + sub_task: Task, + ) -> None: + """Record delegation in guard state and audit trail. - # 4. Record in guard and audit trail + Args: + request: The delegation request. + sub_task: The created sub-task. + """ self._guard.record_delegation( request.delegator_id, request.delegatee_id, @@ -157,18 +188,12 @@ def delegate( original_task_id=request.task.id, delegated_task_id=sub_task.id, ) - - result = DelegationResult( - success=True, - delegated_task=sub_task, - ) logger.debug( DELEGATION_RESULT_SENT, delegator=request.delegator_id, delegatee=request.delegatee_id, success=True, ) - return result def _create_sub_task(self, request: DelegationRequest) -> Task: """Create a new sub-task from a delegation request. @@ -211,10 +236,14 @@ def _create_sub_task(self, request: DelegationRequest) -> Task: budget_limit=original.budget_limit, deadline=original.deadline, max_retries=original.max_retries, + reviewers=original.reviewers, + dependencies=original.dependencies, + artifacts_expected=original.artifacts_expected, + acceptance_criteria=original.acceptance_criteria, ) except ValidationError as exc: logger.exception( - "delegation.sub_task_creation_failed", + DELEGATION_SUB_TASK_FAILED, delegator=request.delegator_id, delegatee=request.delegatee_id, original_task_id=original.id, diff --git a/src/ai_company/observability/events/delegation.py b/src/ai_company/observability/events/delegation.py index 64afd4e3e3..ad79bbec9d 100644 --- a/src/ai_company/observability/events/delegation.py +++ b/src/ai_company/observability/events/delegation.py @@ -8,6 +8,7 @@ DELEGATION_AUTHORITY_DENIED: Final[str] = "delegation.authority_denied" DELEGATION_CREATED: Final[str] = "delegation.created" DELEGATION_RESULT_SENT: Final[str] = "delegation.result_sent" +DELEGATION_SUB_TASK_FAILED: Final[str] = "delegation.sub_task.failed" # Loop prevention DELEGATION_LOOP_BLOCKED: Final[str] = "delegation.loop.blocked" diff --git a/tests/unit/communication/delegation/test_hierarchy.py b/tests/unit/communication/delegation/test_hierarchy.py index 886ab48c94..a585e27cf8 100644 --- a/tests/unit/communication/delegation/test_hierarchy.py +++ b/tests/unit/communication/delegation/test_hierarchy.py @@ -292,6 +292,20 @@ def test_multi_team_lead_keeps_first_supervisor(self) -> None: assert resolver.get_supervisor("dev1") == "shared_lead" assert resolver.get_supervisor("dev2") == "shared_lead" + def test_dept_head_as_team_lead_no_self_cycle(self) -> None: + """Department head also leading a team should not create a self-cycle.""" + dept = Department( + name="Engineering", + head="cto", + budget_percent=50.0, + teams=(Team(name="core", lead="cto", members=("dev1",)),), + ) + company = _make_company(departments=(dept,)) + resolver = HierarchyResolver(company) + # cto should not be their own supervisor + assert resolver.get_supervisor("cto") is None + assert resolver.get_supervisor("dev1") == "cto" + def test_member_with_prior_supervisor_keeps_first(self) -> None: """Member already assigned to a supervisor is not re-assigned.""" dept = Department(