From 398c6cb54a57f7b859e3c05793977dadd4c754e2 Mon Sep 17 00:00:00 2001
From: Glen Gordon <18452241+glenrgordon@users.noreply.github.com>
Date: Thu, 5 Nov 2026 08:19:32 -0600
Subject: [PATCH] fix: keep validation error details JSON-serialisable

`_execute_request` built an `invalid_params` error from `exc.errors()`, whose
`ctx` and `input` fields need not be JSON-native. A handler-origin validation
error -- one raised by a handler constructing a model from arbitrary Python
values -- therefore produced a payload that `json.dumps` refuses inside the
transport's `send`. The response had already been attributed to a request id,
so the `TypeError` escaped through a task with no error handler, nothing was
written, and the peer waited on that id forever.

Serialise the error with pydantic instead: `exc.json(include_context=False)`
drops the exception objects in `ctx` and lets pydantic render `input` -- and
anything a future pydantic adds -- without the SDK choosing how to stringify
someone else's object. Notably pydantic's serialiser never calls `repr`, so an
input with a broken `__repr__` cannot take the expression down; a `fallback=str`
coercion would, and it sits inside an `except` clause where no sibling handler
could catch it.

Two regression tests cover the handler-origin path through a real socket pair,
where the transport actually serialises. Existing coverage was entirely
wire-origin, where every value has already been through `json.loads`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/acp/connection.py |  4 ++-
 tests/test_rpc.py     | 62 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/src/acp/connection.py b/src/acp/connection.py
index 84d009cf..f77b287e 100644
--- a/src/acp/connection.py
+++ b/src/acp/connection.py
@@ -271,7 +271,9 @@ async def _execute_request(self, message: dict[str, Any]) -> dict[str, Any]:
             except RequestError as exc:
                 payload["error"] = exc.to_error_obj()
             except ValidationError as exc:
-                payload["error"] = RequestError.invalid_params({"errors": exc.errors()}).to_error_obj()
+                payload["error"] = RequestError.invalid_params({
+                    "errors": json.loads(exc.json(include_context=False))
+                }).to_error_obj()
             except Exception as exc:
                 logging.exception(
                     "Unhandled error while handling request method=%s",
diff --git a/tests/test_rpc.py b/tests/test_rpc.py
index 43c37fd1..ee8d5c2d 100644
--- a/tests/test_rpc.py
+++ b/tests/test_rpc.py
@@ -5,7 +5,7 @@
 from typing import Any, cast
 
 import pytest
-from pydantic import AnyUrl
+from pydantic import AnyUrl, BaseModel
 
 from acp import (
     AcceptElicitationResponse,
@@ -529,6 +529,66 @@ async def test_invalid_params_results_in_error_response(connect, server):
     assert resp["error"]["code"] == -32602  # invalid params
 
 
+class _HandlerOriginModel(BaseModel):
+    n: int
+
+
+class _NotJsonNative:
+    def __repr__(self) -> str:
+        return "<NotJsonNative>"
+
+
+class _ReprRaises:
+    def __repr__(self) -> str:
+        raise RuntimeError("repr exploded")
+
+
+def _handler_building_model_from(value: Any):
+    async def handler(method: str, params: Any, is_notification: bool) -> Any:
+        _HandlerOriginModel(n=value)  # type: ignore[arg-type]
+        return None
+
+    return handler
+
+
+@pytest.mark.asyncio
+async def test_handler_origin_validation_error_reaches_the_peer(server):
+    # Handler-origin validation error: ``input`` holds a plain Python object, which
+    # ``json.dumps`` in the transport refuses. The reply must still arrive.
+    conn = Connection(_handler_building_model_from(_NotJsonNative()), server.client_writer, server.client_reader)
+    req = {"jsonrpc": "2.0", "id": 1, "method": "thing", "params": {}}
+    server.server_writer.write((json.dumps(req) + "\n").encode())
+    await server.server_writer.drain()
+
+    line = await asyncio.wait_for(server.server_reader.readline(), timeout=2)
+    resp = json.loads(line)
+    assert resp["id"] == 1
+    assert resp["error"]["code"] == -32602
+    # The offending value survives as a diagnostic string rather than being dropped.
+    assert isinstance(resp["error"]["data"]["errors"][0]["input"], str)
+
+    await conn.close()
+
+
+@pytest.mark.asyncio
+async def test_handler_origin_validation_error_survives_an_input_whose_repr_raises(server):
+    # Pins the serialiser: pydantic substitutes a placeholder, while anything that
+    # stringifies the input (e.g. ``fallback=str``) raises inside the ``except``
+    # clause and strands the peer again.
+    conn = Connection(_handler_building_model_from(_ReprRaises()), server.client_writer, server.client_reader)
+    req = {"jsonrpc": "2.0", "id": 2, "method": "thing", "params": {}}
+    server.server_writer.write((json.dumps(req) + "\n").encode())
+    await server.server_writer.drain()
+
+    line = await asyncio.wait_for(server.server_reader.readline(), timeout=2)
+    resp = json.loads(line)
+    assert resp["id"] == 2
+    assert resp["error"]["code"] == -32602
+    assert isinstance(resp["error"]["data"]["errors"][0]["input"], str)
+
+    await conn.close()
+
+
 @pytest.mark.asyncio
 async def test_initialize_accepts_legacy_string_protocol_version(connect, server):
     # Only start agent-side (server) so we can inject raw request from client socket.