diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ad6a3c1b..a66a24084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,13 @@ child plans. Persists `SkeletonMetadata` (ratio, token counts, source decision IDs) on the plan model for auditability. Includes stable fragment ordering, ratio validation with default handling, and compression summary. (#194) +- Wired ACP local facade handlers to live application services. `session.create`/`close` + delegate to `SessionService`; `plan.create`/`execute`/`status`/`diff`/`apply` delegate to + `PlanLifecycleService`; `registry.list_tools` and `registry.list_resources` delegate to + `ToolRegistry` and `ResourceRegistryService`; `event.subscribe` delegates to `AcpEventQueue`. + `context.get` returns a stub pending ACMS pipeline. Added domain-to-ACP error code mapping + (`NOT_FOUND`, `VALIDATION_ERROR`, `INVALID_STATE`, `PLAN_ERROR`, etc.) via `map_domain_error()`. + (#501) - Replaced behave-parallel subprocess-per-feature execution model (342 Python interpreter startups) with in-process execution via behave's `Runner` API. Sequential mode runs all features in a single `Runner.run()` call; parallel mode uses `multiprocessing.Pool` with diff --git a/docs/reference/acp.md b/docs/reference/acp.md index 235b73fd5..5a809cb59 100644 --- a/docs/reference/acp.md +++ b/docs/reference/acp.md @@ -14,7 +14,9 @@ streaming interface. - [Modes of Operation](#modes-of-operation) - [Local Facade](#local-facade) +- [Service Wiring](#service-wiring) - [Operation Routing Table](#operation-routing-table) +- [Error Code Taxonomy](#error-code-taxonomy) - [Server Transport Stub](#server-transport-stub) - [Event Queue](#event-queue) - [Version Negotiation](#version-negotiation) @@ -62,26 +64,107 @@ assert response.status == "ok" --- +## Service Wiring + +Each ACP operation is wired to a concrete application service. Services +are injected via the `services` dict at construction time or registered +later with `register_service()`. When a service key is absent, the +handler falls back to a safe stub response. + +### Service Keys + +| Key | Type | Wired Operations | +|------------------------------|-----------------------------|-------------------------------| +| `session_service` | `SessionService` | `session.create`, `session.close` | +| `plan_lifecycle_service` | `PlanLifecycleService` | `plan.create`, `plan.execute`, `plan.status`, `plan.diff`, `plan.apply` | +| `tool_registry` | `ToolRegistry` | `registry.list_tools` | +| `resource_registry_service` | `ResourceRegistryService` | `registry.list_resources` | +| `event_queue` | `AcpEventQueue` | `event.subscribe` | + +### Wired Operation Details + +| Operation | Service | Service Method | Required Params | +|------------------------|---------------------------|-------------------------|------------------------------------| +| `session.create` | `SessionService` | `create(actor_name=…)` | `actor_name` (optional) | +| `session.close` | `SessionService` | `delete(session_id)` | `session_id` | +| `plan.create` | `PlanLifecycleService` | `use_action(…)` | `action_name` | +| `plan.execute` | `PlanLifecycleService` | `execute_plan(plan_id)` | `plan_id` | +| `plan.status` | `PlanLifecycleService` | `get_plan(plan_id)` | `plan_id` | +| `plan.diff` | `PlanLifecycleService` | `get_plan(plan_id)` | `plan_id` | +| `plan.apply` | `PlanLifecycleService` | `apply_plan(plan_id)` | `plan_id` | +| `registry.list_tools` | `ToolRegistry` | `list_tools(namespace=…)` | `namespace` (optional) | +| `registry.list_resources` | `ResourceRegistryService` | `list_resources(type_name=…)` | `type_name` (optional) | +| `context.get` | *(stub)* | N/A | — | +| `event.subscribe` | `AcpEventQueue` | `subscribe_local(cb)` | — | + +!!! note "context.get" + `context.get` currently returns a stub response (`{"context": {}, "stub": true}`) + pending the completion of the ACMS `ContextAssemblyPipeline`. + +### Example: Wired Facade + +```python +from cleveragents.acp.facade import AcpLocalFacade +from cleveragents.acp.events import AcpEventQueue +from cleveragents.acp.models import AcpRequest + +facade = AcpLocalFacade(services={ + "session_service": my_session_service, + "plan_lifecycle_service": my_plan_lifecycle_service, + "tool_registry": my_tool_registry, + "resource_registry_service": my_resource_registry_service, + "event_queue": AcpEventQueue(), +}) + +response = facade.dispatch( + AcpRequest(operation="plan.status", params={"plan_id": "01J…"}) +) +assert response.status == "ok" +assert "phase" in response.data +``` + +--- + ## Operation Routing Table -| Operation | Response Keys | -|------------------------|-----------------------------------------| -| `session.create` | `session_id`, `status` | -| `session.close` | `status` | -| `plan.create` | `plan_id`, `status` | -| `plan.execute` | `plan_id`, `status` | -| `plan.status` | `plan_id`, `phase` | -| `plan.diff` | `plan_id`, `changes` | -| `plan.apply` | `plan_id`, `status` | -| `registry.list_tools` | `tools` | -| `registry.list_resources` | `resources` | -| `context.get` | `context` | -| `event.subscribe` | `subscription_id`, `status` | +| Operation | Response Keys | +|------------------------|--------------------------------------------------| +| `session.create` | `session_id`, `status` | +| `session.close` | `status` | +| `plan.create` | `plan_id`, `status` | +| `plan.execute` | `plan_id`, `status` | +| `plan.status` | `plan_id`, `phase`, `state` | +| `plan.diff` | `plan_id`, `changes`, `phase` | +| `plan.apply` | `plan_id`, `status` | +| `registry.list_tools` | `tools` (list of `{name, description}`) | +| `registry.list_resources` | `resources` (list of `{resource_id, name, type_name}`) | +| `context.get` | `context`, `stub`, `message` | +| `event.subscribe` | `subscription_id`, `status` | Unknown operations raise `AcpOperationNotFoundError`. --- +## Error Code Taxonomy + +Domain exceptions are mapped to ACP error codes via `map_domain_error()`: + +| ACP Error Code | Domain Exception(s) | Meaning | +|-----------------------|----------------------------------------|----------------------------------| +| `NOT_FOUND` | `ResourceNotFoundError` | Requested entity does not exist | +| `VALIDATION_ERROR` | `ValidationError` | Input failed validation | +| `PLAN_ERROR` | `PlanError` | Plan lifecycle fault | +| `INVALID_STATE` | `BusinessRuleViolation` | State precondition not met | +| `AUTH_ERROR` | `AuthenticationError` | Authentication failure | +| `FORBIDDEN` | `AuthorizationError` | Insufficient permissions | +| `CONFIGURATION_ERROR` | `ConfigurationError` | Configuration missing or invalid | +| `INTERNAL_ERROR` | Any other `CleverAgentsError` or `Exception` | Unexpected server error | + +Error details are returned in the `AcpResponse.error` field as an +`AcpErrorDetail` with the `code` and `message` fields populated. + +--- + ## Server Transport Stub All methods on `AcpHttpTransport` raise `AcpNotAvailableError`: diff --git a/features/acp_facade_wiring.feature b/features/acp_facade_wiring.feature new file mode 100644 index 000000000..69c9f9062 --- /dev/null +++ b/features/acp_facade_wiring.feature @@ -0,0 +1,159 @@ +Feature: ACP local facade wiring to live services + As a client of the ACP local facade + I want operations to route to real application services + So that the facade delegates to live implementations + + # --------------------------------------------------------------- + # Session wiring + # --------------------------------------------------------------- + + Scenario: session.create delegates to SessionService + Given a wired AcpLocalFacade with a mock SessionService + When I dispatch wired operation "session.create" with params {"actor_name": "local/test-actor"} + Then the wired response status should be "ok" + And wired response data key "session_id" equals "MOCK-SESSION-001" + And wired response data key "status" equals "created" + + Scenario: session.close delegates to SessionService + Given a wired AcpLocalFacade with a mock SessionService + When I dispatch wired operation "session.close" with params {"session_id": "MOCK-SESSION-001"} + Then the wired response status should be "ok" + And wired response data key "status" equals "closed" + + Scenario: session.close without session_id returns error + Given a wired AcpLocalFacade with a mock SessionService + When I dispatch wired operation "session.close" with params {} + Then the wired response status should be "error" + + # --------------------------------------------------------------- + # Plan lifecycle wiring + # --------------------------------------------------------------- + + Scenario: plan.create delegates to PlanLifecycleService + Given a wired AcpLocalFacade with a mock PlanLifecycleService + When I dispatch wired operation "plan.create" with params {"action_name": "local/test-action"} + Then the wired response status should be "ok" + And wired response data key "plan_id" equals "MOCK-PLAN-001" + And wired response data key "status" equals "created" + + Scenario: plan.create without action_name returns error + Given a wired AcpLocalFacade with a mock PlanLifecycleService + When I dispatch wired operation "plan.create" with params {} + Then the wired response status should be "error" + + Scenario: plan.execute delegates to PlanLifecycleService + Given a wired AcpLocalFacade with a mock PlanLifecycleService + When I dispatch wired operation "plan.execute" with params {"plan_id": "MOCK-PLAN-001"} + Then the wired response status should be "ok" + And wired response data key "plan_id" equals "MOCK-PLAN-001" + + Scenario: plan.status delegates to PlanLifecycleService + Given a wired AcpLocalFacade with a mock PlanLifecycleService + When I dispatch wired operation "plan.status" with params {"plan_id": "MOCK-PLAN-001"} + Then the wired response status should be "ok" + And wired response data key "plan_id" equals "MOCK-PLAN-001" + And wired response data key "phase" equals "strategize" + + Scenario: plan.diff delegates to PlanLifecycleService + Given a wired AcpLocalFacade with a mock PlanLifecycleService + When I dispatch wired operation "plan.diff" with params {"plan_id": "MOCK-PLAN-001"} + Then the wired response status should be "ok" + And wired response data key "plan_id" equals "MOCK-PLAN-001" + + Scenario: plan.apply delegates to PlanLifecycleService + Given a wired AcpLocalFacade with a mock PlanLifecycleService + When I dispatch wired operation "plan.apply" with params {"plan_id": "MOCK-PLAN-001"} + Then the wired response status should be "ok" + And wired response data key "plan_id" equals "MOCK-PLAN-001" + + # --------------------------------------------------------------- + # Registry wiring + # --------------------------------------------------------------- + + Scenario: registry.list_tools delegates to ToolRegistry + Given a wired AcpLocalFacade with a mock ToolRegistry + When I dispatch wired operation "registry.list_tools" with params {} + Then the wired response status should be "ok" + And wired response data should contain tools list with 2 items + + Scenario: registry.list_resources delegates to ResourceRegistryService + Given a wired AcpLocalFacade with a mock ResourceRegistryService + When I dispatch wired operation "registry.list_resources" with params {} + Then the wired response status should be "ok" + And wired response data should contain resources list with 1 items + + # --------------------------------------------------------------- + # Context stub + # --------------------------------------------------------------- + + Scenario: context.get returns stub pending ACMS pipeline + Given a wired AcpLocalFacade with no services + When I dispatch wired operation "context.get" with params {} + Then the wired response status should be "ok" + And wired response data key "stub" equals "True" + + # --------------------------------------------------------------- + # Event wiring + # --------------------------------------------------------------- + + Scenario: event.subscribe delegates to AcpEventQueue + Given a wired AcpLocalFacade with a mock AcpEventQueue + When I dispatch wired operation "event.subscribe" with params {} + Then the wired response status should be "ok" + And wired response data key "status" equals "subscribed" + And wired response data key "subscription_id" should not be empty + + # --------------------------------------------------------------- + # Error mapping + # --------------------------------------------------------------- + + Scenario: NOT_FOUND error code for missing resources + Given a wired AcpLocalFacade with a raising SessionService for not-found + When I dispatch wired operation "session.close" with params {"session_id": "nonexistent"} + Then the wired response status should be "error" + And wired response error code should be "NOT_FOUND" + + Scenario: VALIDATION_ERROR code for validation failures + Given a wired AcpLocalFacade with a raising service for validation-error + When I dispatch wired operation "plan.create" with params {"action_name": "bad"} + Then the wired response status should be "error" + And wired response error code should be "VALIDATION_ERROR" + + Scenario: PLAN_ERROR code for plan failures + Given a wired AcpLocalFacade with a raising service for plan-error + When I dispatch wired operation "plan.execute" with params {"plan_id": "P1"} + Then the wired response status should be "error" + And wired response error code should be "PLAN_ERROR" + + Scenario: INVALID_STATE code for business rule violations + Given a wired AcpLocalFacade with a raising service for invalid-state + When I dispatch wired operation "plan.apply" with params {"plan_id": "P1"} + Then the wired response status should be "error" + And wired response error code should be "INVALID_STATE" + + # --------------------------------------------------------------- + # Fallback stubs when no service is wired + # --------------------------------------------------------------- + + Scenario: session.create stub when no service wired + Given a wired AcpLocalFacade with no services + When I dispatch wired operation "session.create" with params {} + Then the wired response status should be "ok" + And wired response data key "status" equals "created" + + Scenario: plan.create stub when no service wired + Given a wired AcpLocalFacade with no services + When I dispatch wired operation "plan.create" with params {} + Then the wired response status should be "ok" + And wired response data key "status" equals "created" + + Scenario: registry.list_tools stub when no service wired + Given a wired AcpLocalFacade with no services + When I dispatch wired operation "registry.list_tools" with params {} + Then the wired response status should be "ok" + + Scenario: event.subscribe stub when no service wired + Given a wired AcpLocalFacade with no services + When I dispatch wired operation "event.subscribe" with params {} + Then the wired response status should be "ok" + And wired response data key "status" equals "subscribed" diff --git a/features/steps/acp_facade_wiring_steps.py b/features/steps/acp_facade_wiring_steps.py new file mode 100644 index 000000000..1c13e862e --- /dev/null +++ b/features/steps/acp_facade_wiring_steps.py @@ -0,0 +1,249 @@ +"""Step definitions for ACP facade wiring Behave scenarios. + +All mocks in this file are lightweight test doubles that simulate the +service contracts used by :class:`AcpLocalFacade`. They live here +(inside the test tree) per the project's mock-placement policy. +""" + +from __future__ import annotations + +import json +from typing import Any +from unittest.mock import MagicMock + +from behave import given, then, use_step_matcher, when +from behave.runner import Context + +from cleveragents.acp.events import AcpEventQueue +from cleveragents.acp.facade import AcpLocalFacade +from cleveragents.acp.models import AcpRequest +from cleveragents.core.exceptions import ( + BusinessRuleViolation, + PlanError, + ResourceNotFoundError, + ValidationError, +) + +use_step_matcher("re") + + +# --------------------------------------------------------------------------- +# Mock helpers +# --------------------------------------------------------------------------- + + +class _MockSession: + """Minimal session stub.""" + + def __init__(self, session_id: str = "MOCK-SESSION-001") -> None: + self.session_id = session_id + + +class _MockPlanIdentity: + """Minimal PlanIdentity stub.""" + + def __init__(self, plan_id: str = "MOCK-PLAN-001") -> None: + self.plan_id = plan_id + + +class _MockPlan: + """Minimal plan stub with phase and state.""" + + def __init__(self, plan_id: str = "MOCK-PLAN-001") -> None: + self.identity = _MockPlanIdentity(plan_id) + self.phase = MagicMock() + self.phase.value = "strategize" + self.state = MagicMock() + self.state.value = "queued" + + +class _MockToolSpec: + """Minimal ToolSpec stub.""" + + def __init__(self, name: str, description: str) -> None: + self.name = name + self.description = description + + +class _MockResource: + """Minimal resource stub.""" + + def __init__(self, resource_id: str, name: str, resource_type_name: str) -> None: + self.resource_id = resource_id + self.name = name + self.resource_type_name = resource_type_name + + +def _build_mock_session_service() -> MagicMock: + svc = MagicMock() + svc.create.return_value = _MockSession() + svc.delete.return_value = None + return svc + + +def _build_mock_plan_lifecycle_service() -> MagicMock: + svc = MagicMock() + svc.use_action.return_value = _MockPlan() + svc.execute_plan.return_value = _MockPlan() + svc.get_plan.return_value = _MockPlan() + svc.apply_plan.return_value = _MockPlan() + return svc + + +def _build_mock_tool_registry() -> MagicMock: + registry = MagicMock() + registry.list_tools.return_value = [ + _MockToolSpec("local/tool-a", "Tool A"), + _MockToolSpec("local/tool-b", "Tool B"), + ] + return registry + + +def _build_mock_resource_registry_service() -> MagicMock: + svc = MagicMock() + svc.list_resources.return_value = [ + _MockResource("RES-001", "my-repo", resource_type_name="git-checkout"), + ] + return svc + + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given(r"a wired AcpLocalFacade with a mock SessionService") +def step_wired_facade_session(context: Context) -> None: + context.wired_facade = AcpLocalFacade( + services={"session_service": _build_mock_session_service()} + ) + + +@given(r"a wired AcpLocalFacade with a mock PlanLifecycleService") +def step_wired_facade_plan(context: Context) -> None: + context.wired_facade = AcpLocalFacade( + services={"plan_lifecycle_service": _build_mock_plan_lifecycle_service()} + ) + + +@given(r"a wired AcpLocalFacade with a mock ToolRegistry") +def step_wired_facade_tool_registry(context: Context) -> None: + context.wired_facade = AcpLocalFacade( + services={"tool_registry": _build_mock_tool_registry()} + ) + + +@given(r"a wired AcpLocalFacade with a mock ResourceRegistryService") +def step_wired_facade_resource_registry(context: Context) -> None: + context.wired_facade = AcpLocalFacade( + services={ + "resource_registry_service": (_build_mock_resource_registry_service()) + } + ) + + +@given(r"a wired AcpLocalFacade with a mock AcpEventQueue") +def step_wired_facade_event_queue(context: Context) -> None: + context.wired_facade = AcpLocalFacade(services={"event_queue": AcpEventQueue()}) + + +@given(r"a wired AcpLocalFacade with no services") +def step_wired_facade_no_services(context: Context) -> None: + context.wired_facade = AcpLocalFacade() + + +@given(r"a wired AcpLocalFacade with a raising SessionService for not-found") +def step_wired_facade_not_found(context: Context) -> None: + svc = MagicMock() + svc.delete.side_effect = ResourceNotFoundError( + resource_type="session", resource_id="nonexistent" + ) + context.wired_facade = AcpLocalFacade(services={"session_service": svc}) + + +@given(r"a wired AcpLocalFacade with a raising service for validation-error") +def step_wired_facade_validation_error(context: Context) -> None: + svc = MagicMock() + svc.use_action.side_effect = ValidationError("Invalid action args") + context.wired_facade = AcpLocalFacade(services={"plan_lifecycle_service": svc}) + + +@given(r"a wired AcpLocalFacade with a raising service for plan-error") +def step_wired_facade_plan_error(context: Context) -> None: + svc = MagicMock() + svc.execute_plan.side_effect = PlanError("Plan execution failed") + context.wired_facade = AcpLocalFacade(services={"plan_lifecycle_service": svc}) + + +@given(r"a wired AcpLocalFacade with a raising service for invalid-state") +def step_wired_facade_invalid_state(context: Context) -> None: + svc = MagicMock() + svc.apply_plan.side_effect = BusinessRuleViolation("Cannot apply in current state") + context.wired_facade = AcpLocalFacade(services={"plan_lifecycle_service": svc}) + + +# --------------------------------------------------------------------------- +# When steps +# --------------------------------------------------------------------------- + + +@when( + r'I dispatch wired operation "(?P[^"]+)" ' + r"with params (?P.+)" +) +def step_dispatch_wired(context: Context, operation: str, params_json: str) -> None: + params: dict[str, Any] = json.loads(params_json) + request = AcpRequest(operation=operation, params=params) + context.wired_response = context.wired_facade.dispatch(request) + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then(r'the wired response status should be "(?P[^"]+)"') +def step_wired_status(context: Context, status: str) -> None: + assert context.wired_response.status == status, ( + f"Expected '{status}', got '{context.wired_response.status}'" + ) + + +@then(r'wired response data key "(?P[^"]+)" equals "(?P[^"]+)"') +def step_wired_data_key_value(context: Context, key: str, value: str) -> None: + actual = context.wired_response.data.get(key) + assert str(actual) == value, f"Expected '{value}', got '{actual}'" + + +@then( + r"wired response data should contain tools list " + r"with (?P\d+) items" +) +def step_wired_tools_count(context: Context, count: str) -> None: + tools = context.wired_response.data.get("tools", []) + assert len(tools) == int(count), f"Expected {count} tools, got {len(tools)}" + + +@then( + r"wired response data should contain resources list " + r"with (?P\d+) items" +) +def step_wired_resources_count(context: Context, count: str) -> None: + resources = context.wired_response.data.get("resources", []) + assert len(resources) == int(count), ( + f"Expected {count} resources, got {len(resources)}" + ) + + +@then(r'wired response data key "(?P[^"]+)" should not be empty') +def step_wired_data_key_not_empty(context: Context, key: str) -> None: + actual = context.wired_response.data.get(key) + assert actual, f"Key '{key}' is empty or missing" + + +@then(r'wired response error code should be "(?P[^"]+)"') +def step_wired_error_code(context: Context, code: str) -> None: + assert context.wired_response.error is not None, "No error in response" + assert context.wired_response.error.code == code, ( + f"Expected error code '{code}', got '{context.wired_response.error.code}'" + ) diff --git a/robot/acp_facade_wiring.robot b/robot/acp_facade_wiring.robot new file mode 100644 index 000000000..16f8b7262 --- /dev/null +++ b/robot/acp_facade_wiring.robot @@ -0,0 +1,81 @@ +*** Settings *** +Documentation Integration tests for ACP local facade wiring to live services +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_acp_facade_wiring.py + +*** Test Cases *** +Wired Facade Session Create + [Documentation] Verify session.create routes to SessionService + ${result}= Run Process ${PYTHON} ${HELPER} wired-session-create cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acp-wired-session-create-ok + +Wired Facade Session Close + [Documentation] Verify session.close routes to SessionService + ${result}= Run Process ${PYTHON} ${HELPER} wired-session-close cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acp-wired-session-close-ok + +Wired Facade Plan Create + [Documentation] Verify plan.create routes to PlanLifecycleService + ${result}= Run Process ${PYTHON} ${HELPER} wired-plan-create cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acp-wired-plan-create-ok + +Wired Facade Plan Status + [Documentation] Verify plan.status routes to PlanLifecycleService + ${result}= Run Process ${PYTHON} ${HELPER} wired-plan-status cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acp-wired-plan-status-ok + +Wired Facade Registry Tools + [Documentation] Verify registry.list_tools routes to ToolRegistry + ${result}= Run Process ${PYTHON} ${HELPER} wired-registry-tools cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acp-wired-registry-tools-ok + +Wired Facade Registry Resources + [Documentation] Verify registry.list_resources routes to ResourceRegistryService + ${result}= Run Process ${PYTHON} ${HELPER} wired-registry-resources cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acp-wired-registry-resources-ok + +Wired Facade Context Stub + [Documentation] Verify context.get returns stub while ACMS pipeline is pending + ${result}= Run Process ${PYTHON} ${HELPER} wired-context-stub cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acp-wired-context-stub-ok + +Wired Facade Event Subscribe + [Documentation] Verify event.subscribe routes to AcpEventQueue + ${result}= Run Process ${PYTHON} ${HELPER} wired-event-subscribe cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acp-wired-event-subscribe-ok + +Wired Facade Error Mapping + [Documentation] Verify domain exceptions map to correct ACP error codes + ${result}= Run Process ${PYTHON} ${HELPER} wired-error-mapping cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} acp-wired-error-mapping-ok diff --git a/robot/helper_acp_facade_wiring.py b/robot/helper_acp_facade_wiring.py new file mode 100644 index 000000000..0342f42ed --- /dev/null +++ b/robot/helper_acp_facade_wiring.py @@ -0,0 +1,263 @@ +"""Helper script for acp_facade_wiring.robot integration tests. + +Each subcommand exercises the wired ACP facade with lightweight mock +services and prints a sentinel on success. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from unittest.mock import MagicMock + +# Ensure local source tree is importable +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +from cleveragents.acp.errors import ( # noqa: E402 + map_domain_error, +) +from cleveragents.acp.events import AcpEventQueue # noqa: E402 +from cleveragents.acp.facade import AcpLocalFacade # noqa: E402 +from cleveragents.acp.models import AcpRequest # noqa: E402 +from cleveragents.core.exceptions import ( # noqa: E402 + BusinessRuleViolation, + PlanError, + ResourceNotFoundError, + ValidationError, +) + +# --------------------------------------------------------------------------- +# Mock helpers +# --------------------------------------------------------------------------- + + +class _MockSession: + def __init__(self) -> None: + self.session_id = "INTEG-SESSION-001" + + +class _MockPlanIdentity: + def __init__(self) -> None: + self.plan_id = "INTEG-PLAN-001" + + +class _MockPlan: + def __init__(self) -> None: + self.identity = _MockPlanIdentity() + self.phase = MagicMock() + self.phase.value = "strategize" + self.state = MagicMock() + self.state.value = "queued" + + +class _MockToolSpec: + def __init__(self, name: str, description: str) -> None: + self.name = name + self.description = description + + +class _MockResource: + def __init__(self) -> None: + self.resource_id = "RES-001" + self.name = "test-resource" + self.resource_type_name = "git-checkout" + + +def _mock_session_service() -> MagicMock: + svc = MagicMock() + svc.create.return_value = _MockSession() + svc.delete.return_value = None + return svc + + +def _mock_plan_lifecycle_service() -> MagicMock: + svc = MagicMock() + svc.use_action.return_value = _MockPlan() + svc.execute_plan.return_value = _MockPlan() + svc.get_plan.return_value = _MockPlan() + svc.apply_plan.return_value = _MockPlan() + return svc + + +def _mock_tool_registry() -> MagicMock: + reg = MagicMock() + reg.list_tools.return_value = [ + _MockToolSpec("local/echo", "Echo tool"), + ] + return reg + + +def _mock_resource_registry_service() -> MagicMock: + svc = MagicMock() + svc.list_resources.return_value = [_MockResource()] + return svc + + +# --------------------------------------------------------------------------- +# Subcommands +# --------------------------------------------------------------------------- + + +def wired_session_create() -> None: + """Dispatch session.create through a wired facade.""" + facade = AcpLocalFacade(services={"session_service": _mock_session_service()}) + resp = facade.dispatch(AcpRequest(operation="session.create")) + if resp.status == "ok" and resp.data["session_id"] == "INTEG-SESSION-001": + print("acp-wired-session-create-ok") + else: + print(f"FAIL: {resp}", file=sys.stderr) + sys.exit(1) + + +def wired_session_close() -> None: + """Dispatch session.close through a wired facade.""" + facade = AcpLocalFacade(services={"session_service": _mock_session_service()}) + resp = facade.dispatch( + AcpRequest( + operation="session.close", + params={"session_id": "INTEG-SESSION-001"}, + ) + ) + if resp.status == "ok" and resp.data["status"] == "closed": + print("acp-wired-session-close-ok") + else: + print(f"FAIL: {resp}", file=sys.stderr) + sys.exit(1) + + +def wired_plan_create() -> None: + """Dispatch plan.create through a wired facade.""" + facade = AcpLocalFacade( + services={"plan_lifecycle_service": _mock_plan_lifecycle_service()} + ) + resp = facade.dispatch( + AcpRequest( + operation="plan.create", + params={"action_name": "local/test"}, + ) + ) + if resp.status == "ok" and resp.data["plan_id"] == "INTEG-PLAN-001": + print("acp-wired-plan-create-ok") + else: + print(f"FAIL: {resp}", file=sys.stderr) + sys.exit(1) + + +def wired_plan_status() -> None: + """Dispatch plan.status through a wired facade.""" + facade = AcpLocalFacade( + services={"plan_lifecycle_service": _mock_plan_lifecycle_service()} + ) + resp = facade.dispatch( + AcpRequest( + operation="plan.status", + params={"plan_id": "INTEG-PLAN-001"}, + ) + ) + if resp.status == "ok" and resp.data["phase"] == "strategize": + print("acp-wired-plan-status-ok") + else: + print(f"FAIL: {resp}", file=sys.stderr) + sys.exit(1) + + +def wired_registry_tools() -> None: + """Dispatch registry.list_tools through a wired facade.""" + facade = AcpLocalFacade(services={"tool_registry": _mock_tool_registry()}) + resp = facade.dispatch(AcpRequest(operation="registry.list_tools")) + tools = resp.data.get("tools", []) + if resp.status == "ok" and len(tools) == 1: + print("acp-wired-registry-tools-ok") + else: + print(f"FAIL: {resp}", file=sys.stderr) + sys.exit(1) + + +def wired_registry_resources() -> None: + """Dispatch registry.list_resources through a wired facade.""" + facade = AcpLocalFacade( + services={"resource_registry_service": _mock_resource_registry_service()} + ) + resp = facade.dispatch(AcpRequest(operation="registry.list_resources")) + resources = resp.data.get("resources", []) + if resp.status == "ok" and len(resources) == 1: + print("acp-wired-registry-resources-ok") + else: + print(f"FAIL: {resp}", file=sys.stderr) + sys.exit(1) + + +def wired_context_stub() -> None: + """Dispatch context.get and verify stub response.""" + facade = AcpLocalFacade() + resp = facade.dispatch(AcpRequest(operation="context.get")) + if resp.status == "ok" and resp.data.get("stub") is True: + print("acp-wired-context-stub-ok") + else: + print(f"FAIL: {resp}", file=sys.stderr) + sys.exit(1) + + +def wired_event_subscribe() -> None: + """Dispatch event.subscribe through a wired facade.""" + queue = AcpEventQueue() + facade = AcpLocalFacade(services={"event_queue": queue}) + resp = facade.dispatch(AcpRequest(operation="event.subscribe")) + sub_id = resp.data.get("subscription_id", "") + if resp.status == "ok" and sub_id: + print("acp-wired-event-subscribe-ok") + else: + print(f"FAIL: {resp}", file=sys.stderr) + sys.exit(1) + + +def wired_error_mapping() -> None: + """Verify domain-to-ACP error code mapping.""" + cases: list[tuple[Exception, str]] = [ + (ResourceNotFoundError(resource_type="x", resource_id="1"), "NOT_FOUND"), + (ValidationError("bad"), "VALIDATION_ERROR"), + (PlanError("fail"), "PLAN_ERROR"), + (BusinessRuleViolation("invalid"), "INVALID_STATE"), + ] + for exc, expected_code in cases: + code, _ = map_domain_error(exc) + if code != expected_code: + print( + f"FAIL: expected {expected_code} for {type(exc).__name__}, got {code}", + file=sys.stderr, + ) + sys.exit(1) + print("acp-wired-error-mapping-ok") + + +# --------------------------------------------------------------------------- +# Dispatch +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, object] = { + "wired-session-create": wired_session_create, + "wired-session-close": wired_session_close, + "wired-plan-create": wired_plan_create, + "wired-plan-status": wired_plan_status, + "wired-registry-tools": wired_registry_tools, + "wired-registry-resources": wired_registry_resources, + "wired-context-stub": wired_context_stub, + "wired-event-subscribe": wired_event_subscribe, + "wired-error-mapping": wired_error_mapping, +} + + +def main() -> None: + if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS: + cmds = "|".join(_COMMANDS) + print(f"Usage: {sys.argv[0]} <{cmds}>", file=sys.stderr) + sys.exit(2) + fn = _COMMANDS[sys.argv[1]] + if callable(fn): + fn() + + +if __name__ == "__main__": + main() diff --git a/src/cleveragents/acp/errors.py b/src/cleveragents/acp/errors.py index 20b4c6039..54a90283a 100644 --- a/src/cleveragents/acp/errors.py +++ b/src/cleveragents/acp/errors.py @@ -1,15 +1,47 @@ -"""ACP error hierarchy. +"""ACP error hierarchy and domain-to-ACP error mapping. All ACP-specific exceptions inherit from :class:`AcpError` which itself extends the project-wide :class:`CleverAgentsError`. This mirrors the pattern established by the LSP package. + +The :func:`map_domain_error` function translates domain exceptions into +structured :class:`AcpErrorDetail` instances using a well-defined error +code taxonomy so that clients receive consistent, machine-readable error +responses regardless of which internal service produced the fault. """ from __future__ import annotations from typing import Any -from cleveragents.core.exceptions import CleverAgentsError +from cleveragents.core.exceptions import ( + AuthenticationError, + AuthorizationError, + BusinessRuleViolation, + CleverAgentsError, + ConfigurationError, + PlanError, + ResourceNotFoundError, + ValidationError, +) + +# --------------------------------------------------------------------------- +# ACP error code constants +# --------------------------------------------------------------------------- + +NOT_FOUND: str = "NOT_FOUND" +VALIDATION_ERROR: str = "VALIDATION_ERROR" +INVALID_STATE: str = "INVALID_STATE" +PLAN_ERROR: str = "PLAN_ERROR" +AUTH_ERROR: str = "AUTH_ERROR" +FORBIDDEN: str = "FORBIDDEN" +CONFIGURATION_ERROR: str = "CONFIGURATION_ERROR" +INTERNAL_ERROR: str = "INTERNAL_ERROR" + + +# --------------------------------------------------------------------------- +# Exception classes +# --------------------------------------------------------------------------- class AcpError(CleverAgentsError): @@ -62,9 +94,63 @@ class AcpOperationNotFoundError(AcpError): self.operation = operation +# --------------------------------------------------------------------------- +# Domain-to-ACP error mapping +# --------------------------------------------------------------------------- + + +def map_domain_error(exc: Exception) -> tuple[str, str]: + """Map a domain exception to an ACP error code and message. + + Returns a ``(code, message)`` tuple suitable for constructing an + :class:`~cleveragents.acp.models.AcpErrorDetail`. + + The mapping precedence is intentional: more specific exception types + are checked before their base classes so that, for example, a + ``PlanError`` is classified as ``PLAN_ERROR`` rather than the + generic ``INVALID_STATE`` that would match its ``DomainError`` + ancestor. + + Args: + exc: The exception raised by a domain or application service. + + Returns: + A two-element tuple of ``(error_code, error_message)``. + """ + if not isinstance(exc, Exception): + raise TypeError("exc must be an Exception instance") + + if isinstance(exc, ResourceNotFoundError): + return NOT_FOUND, str(exc) + if isinstance(exc, ValidationError): + return VALIDATION_ERROR, str(exc) + if isinstance(exc, PlanError): + return PLAN_ERROR, str(exc) + if isinstance(exc, BusinessRuleViolation): + return INVALID_STATE, str(exc) + if isinstance(exc, AuthenticationError): + return AUTH_ERROR, str(exc) + if isinstance(exc, AuthorizationError): + return FORBIDDEN, str(exc) + if isinstance(exc, ConfigurationError): + return CONFIGURATION_ERROR, str(exc) + if isinstance(exc, CleverAgentsError): + return INTERNAL_ERROR, str(exc) + return INTERNAL_ERROR, str(exc) + + __all__ = [ + "AUTH_ERROR", + "CONFIGURATION_ERROR", + "FORBIDDEN", + "INTERNAL_ERROR", + "INVALID_STATE", + "NOT_FOUND", + "PLAN_ERROR", + "VALIDATION_ERROR", "AcpError", "AcpNotAvailableError", "AcpOperationNotFoundError", "AcpVersionMismatchError", + "map_domain_error", ] diff --git a/src/cleveragents/acp/facade.py b/src/cleveragents/acp/facade.py index 40748cf0b..025517144 100644 --- a/src/cleveragents/acp/facade.py +++ b/src/cleveragents/acp/facade.py @@ -2,17 +2,34 @@ In local mode every ACP operation maps 1:1 to a method call on the appropriate application service. No serialization, network, or auth. + +Service dependencies are supplied via the ``services`` dict passed to the +constructor. Expected keys: + +| Key | Type | Used by | +|------------------------------|-----------------------------|-----------------------| +| ``session_service`` | ``SessionService`` | session.create/close | +| ``plan_lifecycle_service`` | ``PlanLifecycleService`` | plan.* operations | +| ``tool_registry`` | ``ToolRegistry`` | registry.list_tools | +| ``resource_registry_service``| ``ResourceRegistryService`` | registry.list_resources| +| ``event_queue`` | ``AcpEventQueue`` | event.subscribe | + +When a service is absent the handler falls back to a safe stub +response so the facade never crashes due to missing wiring. """ from __future__ import annotations import time -from typing import Any +from typing import TYPE_CHECKING, Any import structlog from ulid import ULID -from cleveragents.acp.errors import AcpOperationNotFoundError +from cleveragents.acp.errors import ( + AcpOperationNotFoundError, + map_domain_error, +) from cleveragents.acp.models import ( AcpErrorDetail, AcpRequest, @@ -20,6 +37,17 @@ from cleveragents.acp.models import ( AcpVersion, ) +if TYPE_CHECKING: + from cleveragents.acp.events import AcpEventQueue + from cleveragents.application.services.plan_lifecycle_service import ( + PlanLifecycleService, + ) + from cleveragents.application.services.resource_registry_service import ( + ResourceRegistryService, + ) + from cleveragents.domain.models.core.session import SessionService + from cleveragents.tool.registry import ToolRegistry + logger: structlog.stdlib.BoundLogger = structlog.get_logger(__name__) # --------------------------------------------------------------------------- @@ -42,13 +70,44 @@ _SUPPORTED_OPERATIONS: list[str] = [ class AcpLocalFacade: - """Local-mode facade that dispatches ACP operations to services.""" + """Local-mode facade that dispatches ACP operations to services. + + Accepts service dependencies via the ``services`` dict. Each handler + checks for the presence of the required service and delegates to it + when available; otherwise a minimal stub response is returned. + """ def __init__(self, services: dict[str, Any] | None = None) -> None: if services is not None and not isinstance(services, dict): raise TypeError("services must be a dict or None") self._services: dict[str, Any] = dict(services) if services else {} + # ------------------------------------------------------------------ + # Service accessors (typed, nullable) + # ------------------------------------------------------------------ + + @property + def _session_service(self) -> SessionService | None: + return self._services.get("session_service") # type: ignore[return-value] + + @property + def _plan_lifecycle_service(self) -> PlanLifecycleService | None: + svc: object | None = self._services.get("plan_lifecycle_service") + return svc # type: ignore[return-value] + + @property + def _tool_registry(self) -> ToolRegistry | None: + return self._services.get("tool_registry") # type: ignore[return-value] + + @property + def _resource_registry_service(self) -> ResourceRegistryService | None: + svc: object | None = self._services.get("resource_registry_service") + return svc # type: ignore[return-value] + + @property + def _event_queue(self) -> AcpEventQueue | None: + return self._services.get("event_queue") # type: ignore[return-value] + # ------------------------------------------------------------------ # Public API # ------------------------------------------------------------------ @@ -57,7 +116,8 @@ class AcpLocalFacade: """Route an :class:`AcpRequest` to the appropriate handler. Returns an :class:`AcpResponse` with ``status='ok'`` on success or - ``status='error'`` when the operation fails. + ``status='error'`` when the operation fails. Domain exceptions + are mapped to ACP error codes via :func:`map_domain_error`. """ if not isinstance(request, AcpRequest): raise TypeError("request must be an AcpRequest instance") @@ -83,19 +143,21 @@ class AcpLocalFacade: raise except Exception as exc: elapsed = (time.monotonic() - start) * 1000.0 + code, message = map_domain_error(exc) logger.error( "acp.local.dispatch.error", operation=request.operation, request_id=request.request_id, - error=str(exc), + error_code=code, + error=message, ) return AcpResponse( acp_version=AcpVersion.CURRENT, request_id=request.request_id, status="error", error=AcpErrorDetail( - code="INTERNAL_ERROR", - message=str(exc), + code=code, + message=message, ), timing_ms=round(elapsed, 2), ) @@ -146,41 +208,152 @@ class AcpLocalFacade: } # ------------------------------------------------------------------ - # Operation handlers + # Operation handlers — session # ------------------------------------------------------------------ def _handle_session_create(self, params: dict[str, Any]) -> dict[str, Any]: - return {"session_id": str(ULID()), "status": "created"} + svc = self._session_service + if svc is None: + return {"session_id": str(ULID()), "status": "created"} + actor_name: str | None = params.get("actor_name") + session = svc.create(actor_name=actor_name) + return {"session_id": session.session_id, "status": "created"} def _handle_session_close(self, params: dict[str, Any]) -> dict[str, Any]: + svc = self._session_service + if svc is None: + return {"status": "closed"} + session_id = params.get("session_id", "") + if not session_id: + raise ValueError("session_id is required") + svc.delete(session_id) return {"status": "closed"} + # ------------------------------------------------------------------ + # Operation handlers — plan lifecycle + # ------------------------------------------------------------------ + def _handle_plan_create(self, params: dict[str, Any]) -> dict[str, Any]: - return {"plan_id": str(ULID()), "status": "created"} + svc = self._plan_lifecycle_service + if svc is None: + return {"plan_id": str(ULID()), "status": "created"} + action_name = params.get("action_name", "") + if not action_name: + raise ValueError("action_name is required") + plan = svc.use_action( + action_name=action_name, + arguments=params.get("arguments"), + created_by=params.get("created_by"), + ) + return {"plan_id": plan.identity.plan_id, "status": "created"} def _handle_plan_execute(self, params: dict[str, Any]) -> dict[str, Any]: - return {"plan_id": params.get("plan_id", ""), "status": "queued"} + svc = self._plan_lifecycle_service + plan_id = params.get("plan_id", "") + if svc is None: + return {"plan_id": plan_id, "status": "queued"} + if not plan_id: + raise ValueError("plan_id is required") + plan = svc.execute_plan(plan_id) + return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} def _handle_plan_status(self, params: dict[str, Any]) -> dict[str, Any]: - return {"plan_id": params.get("plan_id", ""), "phase": "unknown"} + svc = self._plan_lifecycle_service + plan_id = params.get("plan_id", "") + if svc is None: + return {"plan_id": plan_id, "phase": "unknown"} + if not plan_id: + raise ValueError("plan_id is required") + plan = svc.get_plan(plan_id) + state_value = plan.state.value if plan.state else "unknown" + return { + "plan_id": plan.identity.plan_id, + "phase": plan.phase.value, + "state": state_value, + } def _handle_plan_diff(self, params: dict[str, Any]) -> dict[str, Any]: - return {"plan_id": params.get("plan_id", ""), "changes": []} + svc = self._plan_lifecycle_service + plan_id = params.get("plan_id", "") + if svc is None: + return {"plan_id": plan_id, "changes": []} + if not plan_id: + raise ValueError("plan_id is required") + plan = svc.get_plan(plan_id) + return { + "plan_id": plan.identity.plan_id, + "changes": [], + "phase": plan.phase.value, + } def _handle_plan_apply(self, params: dict[str, Any]) -> dict[str, Any]: - return {"plan_id": params.get("plan_id", ""), "status": "applied"} + svc = self._plan_lifecycle_service + plan_id = params.get("plan_id", "") + if svc is None: + return {"plan_id": plan_id, "status": "applied"} + if not plan_id: + raise ValueError("plan_id is required") + plan = svc.apply_plan(plan_id) + return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} + + # ------------------------------------------------------------------ + # Operation handlers — registry + # ------------------------------------------------------------------ def _handle_registry_list_tools(self, params: dict[str, Any]) -> dict[str, Any]: - return {"tools": []} + registry = self._tool_registry + if registry is None: + return {"tools": []} + namespace: str | None = params.get("namespace") + specs = registry.list_tools(namespace=namespace) + return { + "tools": [{"name": s.name, "description": s.description} for s in specs], + } def _handle_registry_list_resources(self, params: dict[str, Any]) -> dict[str, Any]: - return {"resources": []} + svc = self._resource_registry_service + if svc is None: + return {"resources": []} + type_name: str | None = params.get("type_name") + resources = svc.list_resources(type_name=type_name) + return { + "resources": [ + { + "resource_id": r.resource_id, + "name": r.name, + "type_name": r.resource_type_name, + } + for r in resources + ], + } + + # ------------------------------------------------------------------ + # Operation handlers — context + # ------------------------------------------------------------------ def _handle_context_get(self, params: dict[str, Any]) -> dict[str, Any]: - return {"context": {}} + # TODO: Wire to ACMS ContextAssemblyPipeline once available. + # For now return a stub response indicating the pipeline is pending. + return { + "context": {}, + "stub": True, + "message": "ACMS ContextAssemblyPipeline not yet wired", + } + + # ------------------------------------------------------------------ + # Operation handlers — events + # ------------------------------------------------------------------ def _handle_event_subscribe(self, params: dict[str, Any]) -> dict[str, Any]: - return {"subscription_id": str(ULID()), "status": "subscribed"} + queue = self._event_queue + if queue is None: + return {"subscription_id": str(ULID()), "status": "subscribed"} + + def _noop_callback(_event: Any) -> None: + """Default no-op callback for subscriptions.""" + + sub_id = queue.subscribe_local(_noop_callback) + return {"subscription_id": sub_id, "status": "subscribed"} __all__ = [ diff --git a/vulture_whitelist.py b/vulture_whitelist.py index e081510fe..4d34c2972 100644 --- a/vulture_whitelist.py +++ b/vulture_whitelist.py @@ -518,6 +518,22 @@ compressed_tokens # noqa: B018, F821 original_tokens # noqa: B018, F821 fragment_id # noqa: B018, F821 +# ACP facade wiring — public API error codes and mapping function (#501) +NOT_FOUND # noqa: B018, F821 +VALIDATION_ERROR # noqa: B018, F821 +INVALID_STATE # noqa: B018, F821 +PLAN_ERROR # noqa: B018, F821 +AUTH_ERROR # noqa: B018, F821 +FORBIDDEN # noqa: B018, F821 +CONFIGURATION_ERROR # noqa: B018, F821 +INTERNAL_ERROR # noqa: B018, F821 +map_domain_error # noqa: B018, F821 +_session_service # noqa: B018, F821 +_plan_lifecycle_service # noqa: B018, F821 +_tool_registry # noqa: B018, F821 +_resource_registry_service # noqa: B018, F821 +_event_queue # noqa: B018, F821 + # Server client stubs — public API for server mode (#201) ServerClient # noqa: B018, F821 RemoteExecutionClient # noqa: B018, F821