From b4212bf66912a1c29260e1d90987c58074494e61 Mon Sep 17 00:00:00 2001 From: Beon de Nood Date: Wed, 13 May 2026 13:31:33 -0400 Subject: [PATCH 1/3] fix: update executor and validator for A2A v1.0 protobuf format - Remove preserving_proto_field_name from MessageToDict (use standard camelCase) - Add always_print_fields_with_no_presence for empty field serialization - Update VALID_ROLES to ROLE_USER/ROLE_AGENT (A2A v1 enum format) - Rewrite _validate_parts() for protobuf oneof detection instead of kind discriminator - Fix process manager _find_binary to handle None return from shutil.which --- capiscio_sdk/_rpc/process.py | 2 +- capiscio_sdk/executor.py | 7 +- capiscio_sdk/validators/message.py | 125 ++++++++++++----------------- 3 files changed, 57 insertions(+), 77 deletions(-) diff --git a/capiscio_sdk/_rpc/process.py b/capiscio_sdk/_rpc/process.py index 188b5f8..d7ed1dd 100644 --- a/capiscio_sdk/_rpc/process.py +++ b/capiscio_sdk/_rpc/process.py @@ -52,7 +52,7 @@ def _cleanup_stale_sockets() -> None: pass # Binary download configuration -CORE_VERSION = "2.6.0" +CORE_VERSION = "2.7.0" GITHUB_REPO = "capiscio/capiscio-core" CACHE_DIR = DEFAULT_SOCKET_DIR / "bin" diff --git a/capiscio_sdk/executor.py b/capiscio_sdk/executor.py index 30b697c..5f027a6 100644 --- a/capiscio_sdk/executor.py +++ b/capiscio_sdk/executor.py @@ -92,16 +92,17 @@ async def execute(self, context: RequestContext, event_queue: Any) -> None: await self.delegate.execute(context, event_queue) return - # Convert message to dict for validation (our validators expect dict format) + # Convert message to dict for validation using ProtoJSON conventions + # (camelCase fields, SCREAMING_SNAKE_CASE enums per A2A v1 ADR-001) if hasattr(message, 'model_dump'): message_dict = message.model_dump() elif ProtobufMessage is not None and isinstance(message, ProtobufMessage): - message_dict = MessageToDict(message, preserving_proto_field_name=True) + message_dict = MessageToDict(message, always_print_fields_with_no_presence=True) else: message_dict = {} # Extract identifier for rate limiting - identifier = message_dict.get("message_id") or message.message_id + identifier = message_dict.get("messageId") or message.message_id # Check rate limit if self._rate_limiter and identifier: diff --git a/capiscio_sdk/validators/message.py b/capiscio_sdk/validators/message.py index 554ee72..82ea615 100644 --- a/capiscio_sdk/validators/message.py +++ b/capiscio_sdk/validators/message.py @@ -10,12 +10,15 @@ class MessageValidator: - """Validates A2A message structure and content (per official A2A spec).""" + """Validates A2A message structure and content (per official A2A v1 spec).""" - # Based on official A2A specification from a2a-python SDK + # A2A v1 protocol uses ProtoJSON serialization (ADR-001): + # - camelCase field names + # - SCREAMING_SNAKE_CASE enum values + # - Part uses oneof content (text/raw/url/data), no 'kind' discriminator REQUIRED_FIELDS = ["messageId", "role", "parts"] - VALID_ROLES = ["agent", "user"] - VALID_PART_KINDS = ["text", "file", "data"] + VALID_ROLES = ["ROLE_USER", "ROLE_AGENT"] + VALID_PART_CONTENT_FIELDS = ["text", "raw", "url", "data"] def __init__(self) -> None: """Initialize message validator.""" @@ -115,9 +118,10 @@ def validate(self, message: Dict[str, Any], skip_signature_verification: bool = parts_issues = self._validate_parts(message["parts"]) issues.extend(parts_issues) - # Validate optional fields if present - if "contextId" in message and message["contextId"] is not None: - if not isinstance(message["contextId"], str): + # Validate optional fields if present (A2A v1 ProtoJSON: camelCase) + context_id = message.get("contextId") + if context_id is not None: + if not isinstance(context_id, str): issues.append( ValidationIssue( severity=ValidationSeverity.WARNING, @@ -127,8 +131,9 @@ def validate(self, message: Dict[str, Any], skip_signature_verification: bool = ) ) - if "taskId" in message and message["taskId"] is not None: - if not isinstance(message["taskId"], str): + task_id = message.get("taskId") + if task_id is not None: + if not isinstance(task_id, str): issues.append( ValidationIssue( severity=ValidationSeverity.WARNING, @@ -232,7 +237,7 @@ def _calculate_message_compliance(self, message: Dict[str, Any], issues: List[Va max_score=20, no_duplicate_skill_ids=True, # N/A field_lengths_valid=bool("messageId" in message and message.get("messageId")), - no_ssrf_risks=len([i for i in issues if "SSRF" in i.code or (i.path and "uri" in i.path.lower())]) == 0 + no_ssrf_risks=len([i for i in issues if "SSRF" in i.code or (i.path and "url" in i.path.lower())]) == 0 ) ) @@ -246,7 +251,16 @@ def _calculate_message_compliance(self, message: Dict[str, Any], issues: List[Va ) def _validate_parts(self, parts: List[Any]) -> List[ValidationIssue]: - """Validate message parts array (per A2A spec: TextPart, FilePart, DataPart).""" + """Validate message parts array per A2A v1 spec. + + A2A v1 uses protobuf oneof for Part content: + - text (string) + - raw (bytes, base64 in JSON) + - url (string) + - data (google.protobuf.Value) + + Optional fields: metadata, filename, media_type (mediaType in ProtoJSON). + """ issues: List[ValidationIssue] = [] for i, part in enumerate(parts): @@ -261,100 +275,65 @@ def _validate_parts(self, parts: List[Any]) -> List[ValidationIssue]: ) continue - # Check for 'kind' field (discriminator for Part types) - if "kind" not in part: + # A2A v1: Part uses oneof content — exactly one of text/raw/url/data + present_content = [f for f in self.VALID_PART_CONTENT_FIELDS if f in part] + + if len(present_content) == 0: issues.append( ValidationIssue( severity=ValidationSeverity.ERROR, code="MISSING_FIELD", - message=f"Part {i} missing 'kind' field", - path=f"parts[{i}].kind", + message=f"Part {i} must have one of {self.VALID_PART_CONTENT_FIELDS}", + path=f"parts[{i}]", ) ) continue - kind = part["kind"] - if kind not in self.VALID_PART_KINDS: + if len(present_content) > 1: issues.append( ValidationIssue( severity=ValidationSeverity.WARNING, - code="UNKNOWN_TYPE", - message=f"Part {i} has unknown kind '{kind}' (expected: {self.VALID_PART_KINDS})", - path=f"parts[{i}].kind", + code="MULTIPLE_CONTENT_FIELDS", + message=f"Part {i} has multiple content fields {present_content}; oneof expects exactly one", + path=f"parts[{i}]", ) ) - # Validate based on part type - if kind == "text": - if "text" not in part: - issues.append( - ValidationIssue( - severity=ValidationSeverity.ERROR, - code="MISSING_FIELD", - message=f"TextPart {i} must have 'text' field", - path=f"parts[{i}].text", - ) - ) - elif not isinstance(part["text"], str): + content_field = present_content[0] + + # Validate text content + if content_field == "text": + if not isinstance(part["text"], str): issues.append( ValidationIssue( severity=ValidationSeverity.ERROR, code="INVALID_TYPE", - message=f"TextPart {i} 'text' must be a string", + message=f"Part {i} 'text' must be a string", path=f"parts[{i}].text", ) ) - elif kind == "file": - if "file" not in part: + # Validate raw (base64 bytes) content + elif content_field == "raw": + if not isinstance(part["raw"], str): issues.append( ValidationIssue( severity=ValidationSeverity.ERROR, - code="MISSING_FIELD", - message=f"FilePart {i} must have 'file' field", - path=f"parts[{i}].file", + code="INVALID_TYPE", + message=f"Part {i} 'raw' must be a base64-encoded string", + path=f"parts[{i}].raw", ) ) - else: - file_obj = part["file"] - if not isinstance(file_obj, dict): - issues.append( - ValidationIssue( - severity=ValidationSeverity.ERROR, - code="INVALID_TYPE", - message=f"FilePart {i} 'file' must be an object", - path=f"parts[{i}].file", - ) - ) - else: - # Must have either 'bytes' or 'uri' - if "bytes" not in file_obj and "uri" not in file_obj: - issues.append( - ValidationIssue( - severity=ValidationSeverity.ERROR, - code="MISSING_FIELD", - message=f"FilePart {i} must have either 'bytes' or 'uri'", - path=f"parts[{i}].file", - ) - ) - elif kind == "data": - if "data" not in part: - issues.append( - ValidationIssue( - severity=ValidationSeverity.ERROR, - code="MISSING_FIELD", - message=f"DataPart {i} must have 'data' field", - path=f"parts[{i}].data", - ) - ) - elif not isinstance(part["data"], dict): + # Validate url content + elif content_field == "url": + if not isinstance(part["url"], str): issues.append( ValidationIssue( severity=ValidationSeverity.ERROR, code="INVALID_TYPE", - message=f"DataPart {i} 'data' must be an object", - path=f"parts[{i}].data", + message=f"Part {i} 'url' must be a string", + path=f"parts[{i}].url", ) ) From 6f9ff25da7f771402d858eedc49942865c0b4ece Mon Sep 17 00:00:00 2001 From: Beon de Nood Date: Wed, 13 May 2026 13:31:41 -0400 Subject: [PATCH 2/3] test: update tests for A2A v1.0 compatibility - Update message validator fixtures to use ROLE_USER/ROLE_AGENT and oneof parts - Fix process manager tests to mock workspace binary detection - Add gRPC channel readiness check to MCP service integration tests - Add server health check fixtures to DV integration tests --- tests/integration/test_dv_order_api.py | 2 +- tests/integration/test_dv_sdk.py | 12 ++++++ tests/integration/test_mcp_service.py | 6 ++- tests/unit/test_message_validator.py | 38 ++++++++--------- tests/unit/test_process.py | 59 +++++++++++++++++--------- 5 files changed, 75 insertions(+), 42 deletions(-) diff --git a/tests/integration/test_dv_order_api.py b/tests/integration/test_dv_order_api.py index 0e9af8f..7a1a2b9 100644 --- a/tests/integration/test_dv_order_api.py +++ b/tests/integration/test_dv_order_api.py @@ -228,7 +228,7 @@ def test_create_order_missing_jwk(self, server_health_check): assert resp.status_code in [400, 422], "Order without JWK should be rejected" print("✅ Missing JWK correctly rejected") - def test_create_order_anonymous(self, test_jwk): + def test_create_order_anonymous(self, server_health_check, test_jwk): """Test that DV orders can be created anonymously (per RFC-002 v1.2 Anonymous DV).""" # Anonymous DV allows creation without API key - this is intentional! resp = requests.post( diff --git a/tests/integration/test_dv_sdk.py b/tests/integration/test_dv_sdk.py index 948cb99..22bd7ad 100644 --- a/tests/integration/test_dv_sdk.py +++ b/tests/integration/test_dv_sdk.py @@ -3,6 +3,7 @@ import os import pytest +import requests from capiscio_sdk.dv import create_dv_order, finalize_dv_order, get_dv_order @@ -11,6 +12,17 @@ TEST_SERVER_URL = os.getenv("CAPISCIO_TEST_SERVER", "http://localhost:8080") +@pytest.fixture(scope="module", autouse=True) +def _require_server(): + """Skip all tests if server is not available.""" + try: + resp = requests.get(f"{TEST_SERVER_URL}/health", timeout=2) + if resp.status_code != 200: + pytest.skip(f"Server not healthy at {TEST_SERVER_URL}") + except requests.exceptions.RequestException: + pytest.skip(f"Server not available at {TEST_SERVER_URL}") + + @pytest.fixture def test_jwk(): """Generate a test JWK.""" diff --git a/tests/integration/test_mcp_service.py b/tests/integration/test_mcp_service.py index 195b7ed..b71aa19 100644 --- a/tests/integration/test_mcp_service.py +++ b/tests/integration/test_mcp_service.py @@ -24,9 +24,11 @@ def grpc_client() -> CapiscioRPCClient: client = CapiscioRPCClient(address=GRPC_ADDRESS, auto_start=False) try: client.connect() + # Verify the server is actually reachable (gRPC channels connect lazily) + grpc.channel_ready_future(client._channel).result(timeout=3) yield client - except grpc.RpcError as e: - pytest.skip(f"gRPC server not available at {GRPC_ADDRESS}: {e}") + except (grpc.RpcError, grpc.FutureTimeoutError): + pytest.skip(f"gRPC server not available at {GRPC_ADDRESS}") finally: client.close() diff --git a/tests/unit/test_message_validator.py b/tests/unit/test_message_validator.py index fcc2798..7c070d1 100644 --- a/tests/unit/test_message_validator.py +++ b/tests/unit/test_message_validator.py @@ -11,11 +11,11 @@ def validator(): @pytest.fixture def valid_message(): - """Create a valid test message (per official A2A spec).""" + """Create a valid test message (per A2A v1 ProtoJSON spec).""" return { "messageId": "msg_123", - "role": "user", - "parts": [{"kind": "text", "text": "Hello, world!"}], + "role": "ROLE_USER", + "parts": [{"text": "Hello, world!"}], } @@ -70,7 +70,7 @@ def test_validate_missing_role(validator, valid_message): def test_validate_valid_agent_role(validator, valid_message): """Test validation with valid agent role.""" - valid_message["role"] = "agent" + valid_message["role"] = "ROLE_AGENT" result = validator.validate(valid_message) assert result.success assert result.compliance.total == 100 @@ -92,33 +92,31 @@ def test_validate_empty_parts(validator, valid_message): assert any(i.code == "EMPTY_ARRAY" for i in result.warnings) -def test_validate_part_missing_kind(validator, valid_message): - """Test validation with part missing kind field.""" +def test_validate_part_with_text_content(validator, valid_message): + """Test validation with valid text part (A2A v1 oneof content).""" valid_message["parts"] = [{"text": "Hello"}] result = validator.validate(valid_message) - assert not result.success - assert any(i.code == "MISSING_FIELD" and "kind" in i.path for i in result.errors) + assert result.success -def test_validate_part_unknown_kind(validator, valid_message): - """Test validation with unknown part kind.""" - valid_message["parts"] = [{"kind": "unknown_type", "text": "Hello"}] +def test_validate_part_missing_content(validator, valid_message): + """Test validation with part missing all oneof content fields.""" + valid_message["parts"] = [{"metadata": {}}] # No text/raw/url/data result = validator.validate(valid_message) - assert result.success # Unknown kind is just a warning - assert any(i.code == "UNKNOWN_TYPE" for i in result.warnings) + assert not result.success + assert any(i.code == "MISSING_FIELD" for i in result.errors) -def test_validate_text_part_missing_text(validator, valid_message): - """Test validation with TextPart missing text field.""" - valid_message["parts"] = [{"kind": "text"}] +def test_validate_url_part(validator, valid_message): + """Test validation with valid URL part.""" + valid_message["parts"] = [{"url": "https://example.com/file.txt"}] result = validator.validate(valid_message) - assert not result.success - assert any(i.code == "MISSING_FIELD" and "text" in i.path for i in result.errors) + assert result.success def test_validate_data_part_valid(validator, valid_message): - """Test validation with valid DataPart.""" - valid_message["parts"] = [{"kind": "data", "data": {"key": "value"}}] + """Test validation with valid data part (A2A v1 oneof).""" + valid_message["parts"] = [{"data": {"key": "value"}}] result = validator.validate(valid_message) assert result.success diff --git a/tests/unit/test_process.py b/tests/unit/test_process.py index 6c3247d..158a73e 100644 --- a/tests/unit/test_process.py +++ b/tests/unit/test_process.py @@ -104,38 +104,59 @@ def test_find_binary_system_path(self): """Test find_binary finds binary in system PATH.""" pm = ProcessManager() + original_exists = Path.exists + def mock_exists(self): + if "capiscio-core" in str(self) and "bin" in str(self): + return False + return original_exists(self) + with patch.dict(os.environ, {}, clear=True): - with patch("shutil.which", return_value="/usr/local/bin/capiscio-core"): - result = pm.find_binary() - assert result == Path("/usr/local/bin/capiscio-core") + with patch.object(Path, "exists", mock_exists): + with patch("shutil.which", return_value="/usr/local/bin/capiscio-core"): + result = pm.find_binary() + assert result == Path("/usr/local/bin/capiscio-core") def test_find_binary_cached(self): """Test find_binary finds previously downloaded binary.""" pm = ProcessManager() + original_exists = Path.exists + def mock_exists(self): + if "capiscio-core" in str(self) and "bin" in str(self): + return False + return original_exists(self) + with patch.dict(os.environ, {}, clear=True): - with patch("shutil.which", return_value=None): - with patch.object(ProcessManager, "_get_cached_binary_path") as mock_cached: - mock_path = MagicMock() - mock_path.exists.return_value = True - mock_cached.return_value = mock_path - - result = pm.find_binary() - assert result == mock_path + with patch.object(Path, "exists", mock_exists): + with patch("shutil.which", return_value=None): + with patch.object(ProcessManager, "_get_cached_binary_path") as mock_cached: + mock_path = MagicMock() + mock_path.exists.return_value = True + mock_cached.return_value = mock_path + + result = pm.find_binary() + assert result == mock_path def test_find_binary_not_found(self): """Test find_binary returns None when binary not found.""" pm = ProcessManager() + original_exists = Path.exists + def mock_exists(self): + if "capiscio-core" in str(self) and "bin" in str(self): + return False + return original_exists(self) + with patch.dict(os.environ, {}, clear=True): - with patch("shutil.which", return_value=None): - with patch.object(ProcessManager, "_get_cached_binary_path") as mock_cached: - mock_path = MagicMock() - mock_path.exists.return_value = False - mock_cached.return_value = mock_path - - result = pm.find_binary() - assert result is None + with patch.object(Path, "exists", mock_exists): + with patch("shutil.which", return_value=None): + with patch.object(ProcessManager, "_get_cached_binary_path") as mock_cached: + mock_path = MagicMock() + mock_path.exists.return_value = False + mock_cached.return_value = mock_path + + result = pm.find_binary() + assert result is None @patch("httpx.stream") @patch("os.chmod") From a6c660c618c1c9caee3d0e4b17ae404601aa15e6 Mon Sep 17 00:00:00 2001 From: Beon de Nood Date: Wed, 13 May 2026 13:41:20 -0400 Subject: [PATCH 3/3] fix: address review comments on executor fallback and oneof detection - executor.py: use getattr fallback for message_id/messageId to handle both protobuf and non-protobuf message objects - validators/message.py: filter with part.get(f) is not None instead of f in part to avoid false positives from None-valued keys --- capiscio_sdk/executor.py | 6 +++++- capiscio_sdk/validators/message.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/capiscio_sdk/executor.py b/capiscio_sdk/executor.py index 5f027a6..8ff2157 100644 --- a/capiscio_sdk/executor.py +++ b/capiscio_sdk/executor.py @@ -102,7 +102,11 @@ async def execute(self, context: RequestContext, event_queue: Any) -> None: message_dict = {} # Extract identifier for rate limiting - identifier = message_dict.get("messageId") or message.message_id + identifier = ( + message_dict.get("messageId") + or getattr(message, "message_id", None) + or getattr(message, "messageId", None) + ) # Check rate limit if self._rate_limiter and identifier: diff --git a/capiscio_sdk/validators/message.py b/capiscio_sdk/validators/message.py index 82ea615..532c015 100644 --- a/capiscio_sdk/validators/message.py +++ b/capiscio_sdk/validators/message.py @@ -276,7 +276,7 @@ def _validate_parts(self, parts: List[Any]) -> List[ValidationIssue]: continue # A2A v1: Part uses oneof content — exactly one of text/raw/url/data - present_content = [f for f in self.VALID_PART_CONTENT_FIELDS if f in part] + present_content = [f for f in self.VALID_PART_CONTENT_FIELDS if part.get(f) is not None] if len(present_content) == 0: issues.append(