From badf671b9f8e637df2cc358a13c3d1cc75877c55 Mon Sep 17 00:00:00 2001 From: "opencode-agent[bot]" Date: Thu, 23 Jul 2026 21:29:45 -0300 Subject: [PATCH 1/2] feat(pydantic-acp): recover from auth, expose session bootstrap, propagate real agent errors Hardens the pydantic-acp ACP client/provider so an unusable ACP agent fails legibly instead of silently, and adds the session-bootstrap surface the bridge was missing. Added - Auth recovery: AcpProvider captures initialize's auth_methods and, when session/new is rejected with auth_required (-32000), runs the ACP authenticate flow (an advertised method, or an explicit auth_method_id=) and retries session creation once. Such agents were previously unrecoverable. - Public session bootstrap: AcpProvider.ensure_session() and AcpProvider.set_session_mode() bootstrap a session and select a session mode without sending a prompt turn, so callers no longer reach into the private _ensure_session. - Opt-in AcpProvider(raise_on_empty_turn=True) raises UnexpectedModelBehavior with an ACP-specific diagnostic when a text-output turn yields no visible text. Default False preserves the existing empty-response contract. Fixed - request_prompt propagates the agent's real error (rate limit, auth rejection, upstream API failure) by unwrapping single-child anyio TaskGroup BaseExceptionGroups and dropping TaskGroup __context__ noise, instead of surfacing an opaque "ExceptionGroup: unhandled errors in a TaskGroup". Synchronized workspace bump 1.5.0 -> 1.6.0 with CHANGELOG entry. pydantic-acp client.py is at 100% line and branch coverage; make check (ruff + ty + basedpyright), make tests, make check-coverage, and pre-commit all pass. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CweysdtvG97d28xrbeF9BM --- CHANGELOG.md | 25 + VERSION | 2 +- packages/adapters/langchain-acp/VERSION | 2 +- .../src/langchain_acp/_version.py | 2 +- packages/adapters/pydantic-acp/VERSION | 2 +- .../pydantic-acp/src/pydantic_acp/_version.py | 2 +- .../pydantic-acp/src/pydantic_acp/client.py | 242 +++++++-- packages/helpers/codex-auth-helper/VERSION | 2 +- .../src/codex_auth_helper/_version.py | 2 +- packages/transports/acpremote/VERSION | 2 +- .../acpremote/src/acpremote/_version.py | 2 +- pyproject.toml | 10 +- src/acpkit/_version.py | 2 +- tests/pydantic/test_acp_client_provider.py | 496 +++++++++++++++++- 14 files changed, 734 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dcd38f..9886f54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,31 @@ ACP Kit uses synchronized versions for `acpkit`, `pydantic-acp`, `langchain-acp` ## [Unreleased] +## [1.6.0] - 2026-07-24 + +### Added + +- `pydantic-acp`'s `AcpProvider` recovers from an `auth_required` (`-32000`) + rejection of `session/new` by running the ACP `authenticate` flow — using an + advertised auth method or an explicit `auth_method_id=` — and retrying + session creation once. Agents that gate `session/new` on authentication were + previously unrecoverable. +- Public `AcpProvider.ensure_session()` and `AcpProvider.set_session_mode()` + bootstrap a session and select a session mode without sending a prompt turn, + so callers no longer reach into the private `_ensure_session`. +- Opt-in `AcpProvider(raise_on_empty_turn=True)` raises + `UnexpectedModelBehavior` with an ACP-specific diagnostic when a text-output + turn yields no visible text. The default remains `False`, preserving the + empty-response contract. + +### Fixed + +- `pydantic-acp`'s `request_prompt` propagates the ACP agent's real error + (rate limit, auth rejection, upstream API failure) by unwrapping single-child + anyio TaskGroup `BaseExceptionGroup`s and dropping TaskGroup `__context__` + noise, instead of surfacing an opaque `ExceptionGroup: unhandled errors in a + TaskGroup`. + ## [1.5.0] - 2026-07-24 ### Changed diff --git a/VERSION b/VERSION index bc80560..dc1e644 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.5.0 +1.6.0 diff --git a/packages/adapters/langchain-acp/VERSION b/packages/adapters/langchain-acp/VERSION index bc80560..dc1e644 100644 --- a/packages/adapters/langchain-acp/VERSION +++ b/packages/adapters/langchain-acp/VERSION @@ -1 +1 @@ -1.5.0 +1.6.0 diff --git a/packages/adapters/langchain-acp/src/langchain_acp/_version.py b/packages/adapters/langchain-acp/src/langchain_acp/_version.py index dbcf630..7ce04b8 100644 --- a/packages/adapters/langchain-acp/src/langchain_acp/_version.py +++ b/packages/adapters/langchain-acp/src/langchain_acp/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.5.0" +__version__ = "1.6.0" diff --git a/packages/adapters/pydantic-acp/VERSION b/packages/adapters/pydantic-acp/VERSION index bc80560..dc1e644 100644 --- a/packages/adapters/pydantic-acp/VERSION +++ b/packages/adapters/pydantic-acp/VERSION @@ -1 +1 @@ -1.5.0 +1.6.0 diff --git a/packages/adapters/pydantic-acp/src/pydantic_acp/_version.py b/packages/adapters/pydantic-acp/src/pydantic_acp/_version.py index dbcf630..7ce04b8 100644 --- a/packages/adapters/pydantic-acp/src/pydantic_acp/_version.py +++ b/packages/adapters/pydantic-acp/src/pydantic_acp/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.5.0" +__version__ = "1.6.0" diff --git a/packages/adapters/pydantic-acp/src/pydantic_acp/client.py b/packages/adapters/pydantic-acp/src/pydantic_acp/client.py index ad5eeef..6eda890 100644 --- a/packages/adapters/pydantic-acp/src/pydantic_acp/client.py +++ b/packages/adapters/pydantic-acp/src/pydantic_acp/client.py @@ -10,6 +10,7 @@ from uuid import uuid4 from acp import PROTOCOL_VERSION +from acp.exceptions import RequestError from acp.helpers import text_block from acp.interfaces import Agent as AcpAgent from acp.interfaces import Client as AcpClient @@ -23,6 +24,7 @@ EnvVariable, Implementation, KillTerminalResponse, + NewSessionResponse, PermissionOption, ReadTextFileResponse, ReleaseTerminalResponse, @@ -35,7 +37,7 @@ WaitForTerminalExitResponse, WriteTextFileResponse, ) -from pydantic_ai.exceptions import UserError +from pydantic_ai.exceptions import UnexpectedModelBehavior, UserError from pydantic_ai.messages import ( AudioUrl, BinaryContent, @@ -75,6 +77,9 @@ HistoryMode: TypeAlias = Literal["latest_user", "full"] _DEFAULT_MODEL_NAME = "agent" +_AUTH_REQUIRED_DIAGNOSTIC = ( + "The ACP agent requires authentication (session/new returned auth_required / -32000)" +) AcpPromptRenderer: TypeAlias = Callable[ [Sequence[ModelMessage], ModelRequestParameters], @@ -407,6 +412,31 @@ def _default_client_capabilities() -> ClientCapabilities: ) +def _unwrap_acp_error(exc: BaseException) -> BaseException: + """Return the ACP agent's real error, free of anyio TaskGroup wrapping. + + ACP calls run over a stdio connection whose background reader lives in an + anyio task group. An agent/protocol error (rate limit, auth rejection, + upstream API failure) can therefore reach the caller wrapped as a + single-child :class:`BaseExceptionGroup` ("unhandled errors in a + TaskGroup"), or as the real error carrying that group as its + ``__context__``. Both bury the actual cause. This peels single-child groups + down to the leaf and drops a TaskGroup ``__context__`` so the meaningful + error propagates on its own instead of as an opaque group. + """ + leaf = exc + seen: set[int] = set() + while ( + isinstance(leaf, BaseExceptionGroup) and len(leaf.exceptions) == 1 and id(leaf) not in seen + ): + seen.add(id(leaf)) + leaf = leaf.exceptions[0] + if isinstance(leaf.__context__, BaseExceptionGroup): + leaf.__context__ = None + leaf.__suppress_context__ = True + return leaf + + # --------------------------------------------------------------------------- # AcpProvider # --------------------------------------------------------------------------- @@ -445,6 +475,8 @@ def __init__( prompt_renderer: AcpPromptRenderer | None = None, history_mode: HistoryMode = "latest_user", enable_pydantic_acp_meta: bool | None = None, + auth_method_id: str | None = None, + raise_on_empty_turn: bool = False, ) -> None: """Create a new ACP provider. @@ -477,6 +509,17 @@ def __init__( output. ``None`` auto-enables it only for ACP agents produced by this package; arbitrary ACP agents must not be trusted to implement this private contract. + auth_method_id: ACP ``authenticate`` method id to use when the + agent rejects ``session/new`` with an ``auth_required`` + (``-32000``) error. When ``None`` the provider falls back to + the first authentication method advertised by the agent's + ``initialize`` response. + raise_on_empty_turn: When ``True``, a prompt turn that produces no + visible text for a text-output request raises + :class:`~pydantic_ai.exceptions.UnexpectedModelBehavior` with an + ACP-specific diagnostic instead of returning an empty response. + Defaults to ``False`` to preserve the standard contract where an + empty ACP turn yields a response with no parts. """ self._client = acp_agent @@ -499,10 +542,14 @@ def __init__( if enable_pydantic_acp_meta is None else enable_pydantic_acp_meta ) + self._auth_method_id = auth_method_id + self._raise_on_empty_turn = raise_on_empty_turn self._initialized = False self._session_id: str | None = None self._current_model_name: str | None = None self._model_config_option_available: bool | None = None + self._auth_methods: list[Any] = [] + self._authenticated = False self._session_lock: asyncio.Lock | None = None self._session_lock_loop: asyncio.AbstractEventLoop | None = None @@ -549,6 +596,11 @@ def enable_pydantic_acp_meta(self) -> bool: """Whether this provider opts into private ``pydantic_acp`` ACP metadata.""" return self._enable_pydantic_acp_meta + @property + def raise_on_empty_turn(self) -> bool: + """Whether an empty ACP turn raises instead of returning empty parts.""" + return self._raise_on_empty_turn + async def __aexit__( self, exc_type: type[BaseException] | None, @@ -618,42 +670,56 @@ async def request_prompt( prompt: Sequence[AgentPromptBlock], model_request_parameters: ModelRequestParameters, ) -> _AcpPromptResult: - """Send one prompt turn to the ACP agent and collect its visible text.""" - session_id = await self._ensure_session(model_name=model_name) - start_index = self._host.snapshot_index() - request_meta = None - if self._enable_pydantic_acp_meta: - request_meta = build_structured_output_request_meta(model_request_parameters) - prompt_kwargs: dict[str, Any] = { - "prompt": list(prompt), - "session_id": session_id, - "message_id": uuid4().hex, - } - if request_meta is not None: - prompt_kwargs["_meta"] = request_meta - prompt_response = await self._client.prompt(**prompt_kwargs) - text = await self._agent_message_text_after_prompt( - start_index, - session_id=session_id, - prompt_response=prompt_response, - ) + """Send one prompt turn to the ACP agent and collect its visible text. - response_meta = extract_field_meta(prompt_response) - usage = _usage_from_acp(getattr(prompt_response, "usage", None)) - if not usage.has_values(): - usage = self._host.usage_update_since(start_index, session_id=session_id) - stop_reason = getattr(prompt_response, "stop_reason", None) or getattr( - prompt_response, - "stopReason", - None, - ) - return _AcpPromptResult( - text=text, - usage=usage, - stop_reason=stop_reason, - session_id=session_id, - structured_output=extract_structured_output(response_meta), - ) + Errors raised by the ACP agent (rate limits, auth rejection, upstream + API failures) are propagated with anyio TaskGroup wrapping stripped so + the caller sees the agent's real error rather than an opaque + ``ExceptionGroup: unhandled errors in a TaskGroup``. + """ + try: + session_id = await self._ensure_session(model_name=model_name) + start_index = self._host.snapshot_index() + request_meta = None + if self._enable_pydantic_acp_meta: + request_meta = build_structured_output_request_meta(model_request_parameters) + prompt_kwargs: dict[str, Any] = { + "prompt": list(prompt), + "session_id": session_id, + "message_id": uuid4().hex, + } + if request_meta is not None: + prompt_kwargs["_meta"] = request_meta + prompt_response = await self._client.prompt(**prompt_kwargs) + text = await self._agent_message_text_after_prompt( + start_index, + session_id=session_id, + prompt_response=prompt_response, + ) + + response_meta = extract_field_meta(prompt_response) + usage = _usage_from_acp(getattr(prompt_response, "usage", None)) + if not usage.has_values(): + usage = self._host.usage_update_since(start_index, session_id=session_id) + stop_reason = getattr(prompt_response, "stop_reason", None) or getattr( + prompt_response, + "stopReason", + None, + ) + return _AcpPromptResult( + text=text, + usage=usage, + stop_reason=stop_reason, + session_id=session_id, + structured_output=extract_structured_output(response_meta), + ) + except asyncio.CancelledError: + raise + except BaseException as exc: + cleaned = _unwrap_acp_error(exc) + if cleaned is exc: + raise + raise cleaned from None async def _agent_message_text_after_prompt( self, @@ -680,18 +746,16 @@ async def _agent_message_text_after_prompt( async def _ensure_session(self, *, model_name: str | None) -> str: async with self._get_session_lock(): if not self._initialized: - await self._client.initialize( + init_response = await self._client.initialize( protocol_version=self._protocol_version, client_capabilities=self._client_capabilities or _default_client_capabilities(), client_info=self._client_info, ) + self._auth_methods = list(getattr(init_response, "auth_methods", None) or []) self._initialized = True if self._session_id is None: - session = await self._client.new_session( - cwd=self._cwd, - mcp_servers=list(self._mcp_servers), - ) + session = await self._new_session_with_auth() self._session_id = session.session_id self._model_config_option_available = any( isinstance(option, SessionConfigOptionSelect) and option.id == "model" @@ -719,6 +783,87 @@ async def _ensure_session(self, *, model_name: str | None) -> str: return self._session_id + async def _new_session_with_auth(self) -> NewSessionResponse: + """Create an ACP session, authenticating once if the agent demands it. + + ACP agents may reject ``session/new`` with an ``auth_required`` + (``-32000``) error until the client has authenticated. The base + ``initialize``/``session/new`` handshake never calls ``authenticate``, + so such agents are otherwise unrecoverable. This retries session + creation exactly once after a successful ``authenticate`` call. + """ + try: + return await self._call_new_session() + except RequestError as exc: + if exc.code != RequestError.auth_required().code or self._authenticated: + raise + await self._authenticate() + return await self._call_new_session() + + async def _call_new_session(self) -> Any: + return await self._client.new_session( + cwd=self._cwd, + mcp_servers=list(self._mcp_servers), + ) + + async def _authenticate(self) -> None: + """Run the ACP ``authenticate`` flow using an advertised auth method.""" + authenticate = getattr(self._client, "authenticate", None) + if authenticate is None: + raise UserError( + f"{_AUTH_REQUIRED_DIAGNOSTIC} but the wrapped agent does not expose an " + "'authenticate' method, so the session cannot be established." + ) + method_id = self._auth_method_id or next( + (getattr(method, "id", None) for method in self._auth_methods), + None, + ) + if method_id is None: + advertised = ", ".join( + str(getattr(method, "id", method)) for method in self._auth_methods + ) + raise UserError( + f"{_AUTH_REQUIRED_DIAGNOSTIC} but advertised no usable authentication " + f"method to satisfy it{f' (advertised: {advertised})' if advertised else ''}. " + "Pass auth_method_id=... to AcpProvider, or authenticate the agent " + "out of band." + ) + result = authenticate(method_id=method_id) + if inspect.isawaitable(result): + await result + self._authenticated = True + + async def ensure_session(self, *, model_name: str | None = None) -> str: + """Initialize the ACP connection and return an active session id. + + This is the public entry point for bootstrapping a session without + sending a prompt turn: it performs ``initialize`` (authenticating if the + agent demands it), creates the session, and optionally selects a model. + Callers that need to configure a session up front (for example to set a + session mode) should use this instead of reaching into the private + ``_ensure_session``. + """ + return await self._ensure_session(model_name=model_name) + + async def set_session_mode(self, mode_id: str) -> None: + """Set the ACP session mode, bootstrapping the session if needed. + + ACP session modes (for example the permission mode) are distinct from + the ``model`` session config option and are set via ``session/set_mode``. + This ensures a session exists, then delegates to the wrapped agent's + ``set_session_mode`` when available. + """ + session_id = await self._ensure_session(model_name=None) + set_mode = getattr(self._client, "set_session_mode", None) + if set_mode is None: + raise UserError( + "The ACP agent does not expose 'set_session_mode', so the session " + f"mode {mode_id!r} cannot be selected." + ) + result = set_mode(session_id=session_id, mode_id=mode_id) + if inspect.isawaitable(result): + await result + def _get_session_lock(self) -> asyncio.Lock: loop = asyncio.get_running_loop() if self._session_lock is None or self._session_lock_loop is not loop: @@ -877,7 +1022,18 @@ def _response_parts( "ACP agent did not return pydantic_acp structured output metadata for " "this structured-output request.", ) - return [TextPart(acp_result.text, provider_name=self.system)] if acp_result.text else [] + if acp_result.text: + return [TextPart(acp_result.text, provider_name=self.system)] + if cast(AcpProvider, self._provider).raise_on_empty_turn: + stop_reason = acp_result.stop_reason + raise UnexpectedModelBehavior( + "The ACP agent ended its turn without producing any text output" + f"{f' (stop reason: {stop_reason})' if stop_reason else ''}. " + "This usually means the ACP agent is unauthenticated, its ACP mode is " + "not functioning, or it silently declined the request. Check that the " + "underlying agent is logged in and that its ACP integration is working." + ) + return [] # --------------------------------------------------------------------------- @@ -1088,7 +1244,7 @@ def _agent_supports_pydantic_acp_meta(acp_agent: AcpAgent) -> bool: return getattr(acp_agent, "_pydantic_acp_meta_supported", False) is True -_TEXT_FIELD_NAMES = ("text", "delta", "message", "output_text", "response", "data") +_TEXT_FIELD_NAMES: tuple[str, ...] = ("text", "delta", "message", "output_text", "response", "data") def _extract_text(value: Any) -> str: diff --git a/packages/helpers/codex-auth-helper/VERSION b/packages/helpers/codex-auth-helper/VERSION index bc80560..dc1e644 100644 --- a/packages/helpers/codex-auth-helper/VERSION +++ b/packages/helpers/codex-auth-helper/VERSION @@ -1 +1 @@ -1.5.0 +1.6.0 diff --git a/packages/helpers/codex-auth-helper/src/codex_auth_helper/_version.py b/packages/helpers/codex-auth-helper/src/codex_auth_helper/_version.py index dbcf630..7ce04b8 100644 --- a/packages/helpers/codex-auth-helper/src/codex_auth_helper/_version.py +++ b/packages/helpers/codex-auth-helper/src/codex_auth_helper/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.5.0" +__version__ = "1.6.0" diff --git a/packages/transports/acpremote/VERSION b/packages/transports/acpremote/VERSION index bc80560..dc1e644 100644 --- a/packages/transports/acpremote/VERSION +++ b/packages/transports/acpremote/VERSION @@ -1 +1 @@ -1.5.0 +1.6.0 diff --git a/packages/transports/acpremote/src/acpremote/_version.py b/packages/transports/acpremote/src/acpremote/_version.py index dbcf630..7ce04b8 100644 --- a/packages/transports/acpremote/src/acpremote/_version.py +++ b/packages/transports/acpremote/src/acpremote/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.5.0" +__version__ = "1.6.0" diff --git a/pyproject.toml b/pyproject.toml index f209105..701387f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,12 +23,12 @@ dependencies = [ ] [project.optional-dependencies] -codex = ["codex-auth-helper>=1.5.0,<2.0.0"] -deepagents = ["langchain-acp[deepagents]>=1.5.0,<2.0.0"] -langchain = ["langchain-acp>=1.5.0,<2.0.0"] +codex = ["codex-auth-helper>=1.6.0,<2.0.0"] +deepagents = ["langchain-acp[deepagents]>=1.6.0,<2.0.0"] +langchain = ["langchain-acp>=1.6.0,<2.0.0"] launch = ["uv>=0.8.3"] -pydantic = ["pydantic-acp>=1.5.0,<2.0.0"] -remote = ["acpremote>=1.5.0,<2.0.0"] +pydantic = ["pydantic-acp>=1.6.0,<2.0.0"] +remote = ["acpremote>=1.6.0,<2.0.0"] dev = [ "acpremote", diff --git a/src/acpkit/_version.py b/src/acpkit/_version.py index dbcf630..7ce04b8 100644 --- a/src/acpkit/_version.py +++ b/src/acpkit/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.5.0" +__version__ = "1.6.0" diff --git a/tests/pydantic/test_acp_client_provider.py b/tests/pydantic/test_acp_client_provider.py index 99de6c7..0541914 100644 --- a/tests/pydantic/test_acp_client_provider.py +++ b/tests/pydantic/test_acp_client_provider.py @@ -22,6 +22,7 @@ AgentCapabilities, AgentMessageChunk, AllowedOutcome, + AuthMethodAgent, ClientCapabilities, ElicitationFormSessionMode, ElicitationMode, @@ -51,7 +52,7 @@ has_structured_output_request, ) from pydantic_ai import Agent -from pydantic_ai.exceptions import UserError +from pydantic_ai.exceptions import UnexpectedModelBehavior, UserError from pydantic_ai.messages import ( BinaryContent, ImageUrl, @@ -2162,3 +2163,496 @@ def test_acp_model_profile_disables_tool_and_structured_output_support() -> None assert profile["supports_json_object_output"] is False assert profile["supports_image_output"] is False assert profile["supported_native_tools"] == frozenset() + + +# --------------------------------------------------------------------------- +# Authentication recovery, explicit session bootstrap, and empty-turn handling +# --------------------------------------------------------------------------- + + +class AuthRequiredACPAgent(EchoACPAgent): + """An ACP agent that rejects ``session/new`` until ``authenticate`` runs.""" + + def __init__(self, *, auth_methods: list[Any], fail_times: int = 1) -> None: + super().__init__() + self._auth_methods = auth_methods + self._fail_times = fail_times + self.authenticated_with: list[str] = [] + self.new_session_calls = 0 + + async def initialize( + self, + protocol_version: int, + client_capabilities: ClientCapabilities | None = None, + client_info: Implementation | None = None, + **kwargs: Any, + ) -> InitializeResponse: + del client_capabilities, client_info, kwargs + self.initialized_protocols.append(protocol_version) + return InitializeResponse( + protocol_version=protocol_version, + agent_info=Implementation(name="auth-acp-agent", version="test"), + agent_capabilities=AgentCapabilities(), + auth_methods=self._auth_methods, + ) + + async def new_session( + self, + cwd: str, + mcp_servers: list[Any] | None = None, + **kwargs: Any, + ) -> NewSessionResponse: + self.new_session_calls += 1 + if self.new_session_calls <= self._fail_times: + raise RequestError.auth_required() + return await super().new_session(cwd, mcp_servers, **kwargs) + + async def authenticate(self, method_id: str, **kwargs: Any) -> None: + del kwargs + self.authenticated_with.append(method_id) + + +async def test_acp_provider_authenticates_and_retries_session_new_on_auth_required() -> None: + agent = AuthRequiredACPAgent( + auth_methods=[AuthMethodAgent(id="login", name="Login", description="Log in")], + ) + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + + session_id = await provider.ensure_session() + + assert session_id == "session-1" + assert agent.new_session_calls == 2 + assert agent.authenticated_with == ["login"] + + +async def test_acp_provider_uses_explicit_auth_method_id_when_configured() -> None: + agent = AuthRequiredACPAgent( + auth_methods=[AuthMethodAgent(id="login", name="Login", description="Log in")], + ) + provider = AcpProvider( + acp_agent=cast(AcpAgent, agent), cwd="/workspace", auth_method_id="custom" + ) + + await provider.ensure_session() + + assert agent.authenticated_with == ["custom"] + + +async def test_acp_provider_auth_required_without_any_method_raises_userror() -> None: + agent = AuthRequiredACPAgent(auth_methods=[]) + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + + with pytest.raises(UserError, match="authentication"): + await provider.ensure_session() + + +async def test_acp_provider_auth_required_without_authenticate_support_raises_userror() -> None: + class NoAuthenticateACPAgent(EchoACPAgent): + async def new_session( + self, + cwd: str, + mcp_servers: list[Any] | None = None, + **kwargs: Any, + ) -> NewSessionResponse: + del cwd, mcp_servers, kwargs + raise RequestError.auth_required() + + provider = AcpProvider(acp_agent=cast(AcpAgent, NoAuthenticateACPAgent()), cwd="/workspace") + + with pytest.raises(UserError, match="does not expose an 'authenticate' method"): + await provider.ensure_session() + + +async def test_acp_provider_ensure_session_bootstraps_without_a_prompt() -> None: + agent = EchoACPAgent() + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + + session_id = await provider.ensure_session() + + assert session_id == "session-1" + assert provider.session_id == "session-1" + assert agent.initialized_protocols == [PROTOCOL_VERSION] + assert agent.prompts == [] + + +async def test_acp_provider_set_session_mode_delegates_to_agent() -> None: + mode_calls: list[tuple[str, str]] = [] + + class ModeACPAgent(EchoACPAgent): + async def set_session_mode( + self, + session_id: str, + mode_id: str, + **kwargs: Any, + ) -> None: + del kwargs + mode_calls.append((session_id, mode_id)) + + provider = AcpProvider(acp_agent=cast(AcpAgent, ModeACPAgent()), cwd="/workspace") + + await provider.set_session_mode("default") + + assert mode_calls == [("session-1", "default")] + + +async def test_acp_provider_set_session_mode_without_support_raises_userror() -> None: + provider = AcpProvider(acp_agent=cast(AcpAgent, EchoACPAgent()), cwd="/workspace") + + with pytest.raises(UserError, match="set_session_mode"): + await provider.set_session_mode("default") + + +async def test_empty_turn_returns_empty_parts_by_default() -> None: + provider = AcpProvider(acp_agent=cast(AcpAgent, NoHandshakeACPAgent()), cwd="/workspace") + model = provider.model() + + response = await model.request( + [ModelRequest(parts=[UserPromptPart("hello")])], + None, + ModelRequestParameters(), + ) + + assert response.parts == [] + + +async def test_empty_turn_raises_acp_specific_error_when_opted_in() -> None: + provider = AcpProvider( + acp_agent=cast(AcpAgent, NoHandshakeACPAgent()), + cwd="/workspace", + raise_on_empty_turn=True, + ) + model = provider.model() + + with pytest.raises(UnexpectedModelBehavior, match="ACP agent ended its turn"): + await model.request( + [ModelRequest(parts=[UserPromptPart("hello")])], + None, + ModelRequestParameters(), + ) + + +# --------------------------------------------------------------------------- +# Additional coverage: auth edge cases, session bootstrap, and empty turns +# --------------------------------------------------------------------------- + + +async def test_acp_provider_does_not_retry_authenticate_after_it_already_ran() -> None: + """A second ``auth_required`` after authenticating must propagate, not loop.""" + agent = AuthRequiredACPAgent( + auth_methods=[AuthMethodAgent(id="login", name="Login", description="Log in")], + fail_times=2, + ) + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + + with pytest.raises(RequestError) as exc_info: + await provider.ensure_session() + + assert exc_info.value.code == RequestError.auth_required().code + assert agent.new_session_calls == 2 + assert agent.authenticated_with == ["login"] + + +async def test_acp_provider_non_auth_request_error_propagates_without_authenticating() -> None: + class OtherErrorACPAgent(EchoACPAgent): + async def new_session( + self, + cwd: str, + mcp_servers: list[Any] | None = None, + **kwargs: Any, + ) -> NewSessionResponse: + del cwd, mcp_servers, kwargs + raise RequestError.internal_error() + + provider = AcpProvider(acp_agent=cast(AcpAgent, OtherErrorACPAgent()), cwd="/workspace") + + with pytest.raises(RequestError) as exc_info: + await provider.ensure_session() + + assert exc_info.value.code == RequestError.internal_error().code + + +async def test_acp_provider_uses_first_advertised_auth_method_when_multiple_exist() -> None: + agent = AuthRequiredACPAgent( + auth_methods=[ + AuthMethodAgent(id="oauth", name="OAuth", description="OAuth login"), + AuthMethodAgent(id="apikey", name="API Key", description="API key login"), + ], + ) + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + + await provider.ensure_session() + + assert agent.authenticated_with == ["oauth"] + + +async def test_acp_provider_supports_synchronous_authenticate() -> None: + # A *synchronous* authenticate() exercises the non-awaitable branch of + # AcpProvider._authenticate. It subclasses EchoACPAgent (which has no + # authenticate) rather than AuthRequiredACPAgent, so the sync method is a + # fresh definition, not an invalid override of an async one. + class SyncAuthenticateACPAgent(EchoACPAgent): + def __init__(self) -> None: + super().__init__() + self.authenticated_with: list[str] = [] + self.new_session_calls = 0 + + async def initialize( + self, + protocol_version: int, + client_capabilities: ClientCapabilities | None = None, + client_info: Implementation | None = None, + **kwargs: Any, + ) -> InitializeResponse: + del client_capabilities, client_info, kwargs + self.initialized_protocols.append(protocol_version) + return InitializeResponse( + protocol_version=protocol_version, + agent_info=Implementation(name="sync-auth-agent", version="test"), + agent_capabilities=AgentCapabilities(), + auth_methods=[AuthMethodAgent(id="login", name="Login", description="Log in")], + ) + + async def new_session( + self, + cwd: str, + mcp_servers: list[Any] | None = None, + **kwargs: Any, + ) -> NewSessionResponse: + self.new_session_calls += 1 + if self.new_session_calls == 1: + raise RequestError.auth_required() + return await super().new_session(cwd, mcp_servers, **kwargs) + + def authenticate(self, method_id: str, **kwargs: Any) -> None: + del kwargs + self.authenticated_with.append(method_id) + + agent = SyncAuthenticateACPAgent() + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + + session_id = await provider.ensure_session() + + assert session_id == "session-1" + assert agent.new_session_calls == 2 + assert agent.authenticated_with == ["login"] + + +async def test_acp_provider_auth_required_without_any_method_error_omits_advertised_clause() -> ( + None +): + agent = AuthRequiredACPAgent(auth_methods=[]) + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + + with pytest.raises(UserError) as exc_info: + await provider.ensure_session() + + assert "(advertised:" not in str(exc_info.value) + + +async def test_acp_provider_auth_required_with_unusable_advertised_methods_raises_userror() -> None: + class AuthMethodsWithoutIdACPAgent(EchoACPAgent): + async def initialize( + self, + protocol_version: int, + client_capabilities: ClientCapabilities | None = None, + client_info: Implementation | None = None, + **kwargs: Any, + ) -> Any: + del client_capabilities, client_info, kwargs + self.initialized_protocols.append(protocol_version) + return SimpleNamespace(auth_methods=[SimpleNamespace(name="mystery")]) + + async def new_session( + self, + cwd: str, + mcp_servers: list[Any] | None = None, + **kwargs: Any, + ) -> NewSessionResponse: + del cwd, mcp_servers, kwargs + raise RequestError.auth_required() + + async def authenticate(self, method_id: str, **kwargs: Any) -> None: + del method_id, kwargs + + provider = AcpProvider( + acp_agent=cast(AcpAgent, AuthMethodsWithoutIdACPAgent()), cwd="/workspace" + ) + + with pytest.raises(UserError, match="advertised no usable authentication"): + await provider.ensure_session() + + +async def test_acp_provider_ensure_session_forwards_model_name() -> None: + agent = EchoACPAgent() + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + + await provider.ensure_session(model_name="agent") + + assert agent.session_models == [("session-1", "agent")] + + +async def test_acp_provider_set_session_mode_supports_synchronous_agent_method() -> None: + mode_calls: list[tuple[str, str]] = [] + + class SyncModeACPAgent(EchoACPAgent): + def set_session_mode(self, session_id: str, mode_id: str, **kwargs: Any) -> None: + del kwargs + mode_calls.append((session_id, mode_id)) + + provider = AcpProvider(acp_agent=cast(AcpAgent, SyncModeACPAgent()), cwd="/workspace") + + await provider.set_session_mode("default") + + assert mode_calls == [("session-1", "default")] + + +async def test_acp_provider_set_session_mode_reuses_session_created_by_prior_prompt() -> None: + class ModeACPAgent(EchoACPAgent): + def __init__(self) -> None: + super().__init__() + self.mode_calls: list[tuple[str, str]] = [] + + async def set_session_mode( + self, + session_id: str, + mode_id: str, + **kwargs: Any, + ) -> None: + del kwargs + self.mode_calls.append((session_id, mode_id)) + + agent = ModeACPAgent() + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + model = provider.model() + + await model.request( + [ModelRequest(parts=[UserPromptPart("hello")])], + None, + ModelRequestParameters(), + ) + await provider.set_session_mode("default") + + assert agent.mode_calls == [(provider.session_id, "default")] + assert agent.session_cwds == ["/workspace"] + + +def test_acp_provider_raise_on_empty_turn_property_reflects_constructor_arg() -> None: + default_provider = AcpProvider(acp_agent=cast(AcpAgent, EchoACPAgent()), cwd="/workspace") + assert default_provider.raise_on_empty_turn is False + + opted_in_provider = AcpProvider( + acp_agent=cast(AcpAgent, EchoACPAgent()), + cwd="/workspace", + raise_on_empty_turn=True, + ) + assert opted_in_provider.raise_on_empty_turn is True + + +async def test_empty_turn_raises_with_stop_reason_in_message_when_available() -> None: + class RefusalACPAgent(NoHandshakeACPAgent): + async def prompt( + self, + prompt: list[Any], + session_id: str, + message_id: str | None = None, + **kwargs: Any, + ) -> PromptResponse: + del prompt, session_id, message_id, kwargs + return PromptResponse(stop_reason="refusal") + + provider = AcpProvider( + acp_agent=cast(AcpAgent, RefusalACPAgent()), + cwd="/workspace", + raise_on_empty_turn=True, + ) + model = provider.model() + + with pytest.raises(UnexpectedModelBehavior, match=r"stop reason: refusal"): + await model.request( + [ModelRequest(parts=[UserPromptPart("hello")])], + None, + ModelRequestParameters(), + ) + + +# --------------------------------------------------------------------------- +# Agent error propagation (anyio TaskGroup unwrapping) +# --------------------------------------------------------------------------- + + +def test_unwrap_acp_error_peels_single_child_group() -> None: + inner = RequestError(-32000, "Rate limited") + group = BaseExceptionGroup("unhandled errors in a TaskGroup", [inner]) + assert client_module._unwrap_acp_error(group) is inner + + +def test_unwrap_acp_error_peels_nested_single_child_groups() -> None: + inner = RequestError(-32000, "Rate limited") + nested = BaseExceptionGroup("outer", [BaseExceptionGroup("inner", [inner])]) + assert client_module._unwrap_acp_error(nested) is inner + + +def test_unwrap_acp_error_drops_taskgroup_context_noise() -> None: + err = RuntimeError("boom") + err.__context__ = BaseExceptionGroup("unhandled errors in a TaskGroup", [ValueError("x")]) + cleaned = client_module._unwrap_acp_error(err) + assert cleaned is err + assert cleaned.__context__ is None + assert cleaned.__suppress_context__ is True + + +def test_unwrap_acp_error_leaves_plain_exception_untouched() -> None: + plain = ValueError("z") + assert client_module._unwrap_acp_error(plain) is plain + + +def test_unwrap_acp_error_keeps_multi_child_group() -> None: + group = BaseExceptionGroup("many", [ValueError("a"), RequestError(-32000, "b")]) + assert client_module._unwrap_acp_error(group) is group + + +async def test_request_prompt_unwraps_taskgroup_error_from_the_agent() -> None: + class GroupErrorACPAgent(EchoACPAgent): + async def prompt( + self, + prompt: list[Any], + session_id: str, + message_id: str | None = None, + **kwargs: Any, + ) -> PromptResponse: + del prompt, session_id, message_id, kwargs + raise BaseExceptionGroup( + "unhandled errors in a TaskGroup", + [RequestError(-32000, "Rate limited")], + ) + + provider = AcpProvider(acp_agent=cast(AcpAgent, GroupErrorACPAgent()), cwd="/workspace") + + with pytest.raises(RequestError, match="Rate limited"): + await provider.request_prompt( + model_name=None, + prompt=[text_block("hi")], + model_request_parameters=ModelRequestParameters(), + ) + + +async def test_request_prompt_reraises_cancellederror_untouched() -> None: + class CancellingACPAgent(EchoACPAgent): + async def prompt( + self, + prompt: list[Any], + session_id: str, + message_id: str | None = None, + **kwargs: Any, + ) -> PromptResponse: + del prompt, session_id, message_id, kwargs + raise asyncio.CancelledError + + provider = AcpProvider(acp_agent=cast(AcpAgent, CancellingACPAgent()), cwd="/workspace") + + with pytest.raises(asyncio.CancelledError): + await provider.request_prompt( + model_name=None, + prompt=[text_block("hi")], + model_request_parameters=ModelRequestParameters(), + ) From f91d10c827213a67c50294484362763e20578e4a Mon Sep 17 00:00:00 2001 From: Mert <62549656+fswair@users.noreply.github.com> Date: Fri, 24 Jul 2026 16:47:21 +0300 Subject: [PATCH 2/2] fix(pydantic-acp): harden session recovery for 1.5.1 --- .agents/skills/pydantic-acp/SKILL.md | 19 +++ CHANGELOG.md | 15 ++- COVERAGE | 4 +- README.md | 9 ++ VERSION | 2 +- docs/compatibility-matrix-template.md | 6 +- docs/pydantic-acp.md | 32 +++++ packages/adapters/langchain-acp/VERSION | 2 +- .../src/langchain_acp/_version.py | 2 +- packages/adapters/pydantic-acp/README.md | 12 ++ packages/adapters/pydantic-acp/VERSION | 2 +- .../pydantic-acp/src/pydantic_acp/_version.py | 2 +- .../pydantic-acp/src/pydantic_acp/client.py | 46 ++++--- .../pydantic-acp/src/pydantic_acp/factory.py | 8 ++ packages/helpers/codex-auth-helper/VERSION | 2 +- .../src/codex_auth_helper/_version.py | 2 +- packages/transports/acpremote/VERSION | 2 +- .../acpremote/src/acpremote/_version.py | 2 +- pyproject.toml | 10 +- src/acpkit/_version.py | 2 +- tests/pydantic/test_acp_client_provider.py | 120 +++++++++++++----- 21 files changed, 222 insertions(+), 79 deletions(-) diff --git a/.agents/skills/pydantic-acp/SKILL.md b/.agents/skills/pydantic-acp/SKILL.md index 8fda35d..160ee12 100644 --- a/.agents/skills/pydantic-acp/SKILL.md +++ b/.agents/skills/pydantic-acp/SKILL.md @@ -364,6 +364,20 @@ agent = Agent(model) Do not pass arbitrary CLI commands here; the process must be an ACP server. Each command-backed model owns one child ACP process and should be reused for repeated subagent calls. +`create_acp_model(...)` and `AcpProvider(...)` recover from an ACP +`auth_required` response by authenticating with the first advertised +`AuthMethodAgent` and retrying `session/new` once. Use `auth_method_id=...` only +when a specific method has already been prepared. `EnvVarAuthMethod` and +`TerminalAuthMethod` require host-owned credential injection or terminal +execution and must not be selected automatically. + +Use `AcpProvider.ensure_session()` to initialize and create a session without a +prompt. Use `AcpProvider.set_session_mode(...)` to bootstrap that session and +select a mode before the first model turn. Set `raise_on_empty_turn=True` on the +provider or factory when a silent text turn must raise +`UnexpectedModelBehavior`; the backward-compatible default returns an empty +response. + Use `AcpProvider(acp_agent=...)` directly only when provider ownership, host delegation, or lower level lifecycle control matters. @@ -464,6 +478,11 @@ Stay in this skill when the main issue is: `plan`; the runtime must fall back to full updates otherwise. - Accept and persist `AcpMcpServer` session payloads, but do not advertise ACP MCP transport capability until the SDK exposes a public router. +- Automatically authenticate only with `AuthMethodAgent`. Environment-variable + and terminal auth methods require client-owned setup before `authenticate` + can be called. +- Unwrap only single-child groups whose message identifies an anyio TaskGroup; + preserve unrelated exception groups and cancellation. - Do not describe `pydantic-acp` as transport. - Do not promise ACP state the active `pydantic_ai.Agent` cannot honor. diff --git a/CHANGELOG.md b/CHANGELOG.md index 9886f54..2e1af82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,15 +5,16 @@ ACP Kit uses synchronized versions for `acpkit`, `pydantic-acp`, `langchain-acp` ## [Unreleased] -## [1.6.0] - 2026-07-24 +## [1.5.1] - 2026-07-24 ### Added - `pydantic-acp`'s `AcpProvider` recovers from an `auth_required` (`-32000`) rejection of `session/new` by running the ACP `authenticate` flow — using an - advertised auth method or an explicit `auth_method_id=` — and retrying - session creation once. Agents that gate `session/new` on authentication were - previously unrecoverable. + advertised agent-managed method or an explicit `auth_method_id=` — and + retrying session creation once. Environment-variable and terminal methods + remain client-owned because they require credential injection or an + interactive process. - Public `AcpProvider.ensure_session()` and `AcpProvider.set_session_mode()` bootstrap a session and select a session mode without sending a prompt turn, so callers no longer reach into the private `_ensure_session`. @@ -26,9 +27,9 @@ ACP Kit uses synchronized versions for `acpkit`, `pydantic-acp`, `langchain-acp` - `pydantic-acp`'s `request_prompt` propagates the ACP agent's real error (rate limit, auth rejection, upstream API failure) by unwrapping single-child - anyio TaskGroup `BaseExceptionGroup`s and dropping TaskGroup `__context__` - noise, instead of surfacing an opaque `ExceptionGroup: unhandled errors in a - TaskGroup`. + anyio TaskGroup exception wrappers and dropping matching TaskGroup + `__context__` noise, while preserving unrelated aggregate errors and + cancellation. ## [1.5.0] - 2026-07-24 diff --git a/COVERAGE b/COVERAGE index 721f3c3..ad42586 100644 --- a/COVERAGE +++ b/COVERAGE @@ -1,2 +1,2 @@ -Line coverage: 100.00% (10329 / 10329) -Branch coverage: 100.00% (3434 / 3434) +Line coverage: 100.00% (10398 / 10398) +Branch coverage: 100.00% (3456 / 3456) diff --git a/README.md b/README.md index e1ace70..2211dbf 100644 --- a/README.md +++ b/README.md @@ -303,10 +303,17 @@ model = create_acp_model( acp_command=("npx", "@zed-industries/codex-acp"), cwd="/workspace", stderr_mode="inherit", + raise_on_empty_turn=True, ) agent = Agent(model) ``` +When `session/new` reports `auth_required`, the provider authenticates with the +first agent-managed method advertised by the ACP agent and retries once. Pass +`auth_method_id="..."` to `create_acp_model(...)` or `AcpProvider(...)` to +select a specific prepared method. Environment-variable and terminal methods +still require their client-side credential or terminal setup. + For lower-level ownership, construct the provider directly: ```python @@ -315,6 +322,8 @@ from pydantic_acp import AcpProvider # `remote_acp_agent` can be any object implementing the ACP Agent interface. provider = AcpProvider(acp_agent=remote_acp_agent, cwd="/workspace") +session_id = await provider.ensure_session() +await provider.set_session_mode("review") model = provider.model() agent = Agent(model) diff --git a/VERSION b/VERSION index dc1e644..26ca594 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.6.0 +1.5.1 diff --git a/docs/compatibility-matrix-template.md b/docs/compatibility-matrix-template.md index 9c43825..409f123 100644 --- a/docs/compatibility-matrix-template.md +++ b/docs/compatibility-matrix-template.md @@ -64,8 +64,10 @@ manifest = CompatibilityManifest( rationale='The runtime keeps the hook seam but does not expose it to ACP clients.', ), 'authenticate': SurfaceSupport( - status='planned', - rationale='No auth handshake has been added yet.', + status='implemented', + owner='provider', + mapping='AcpProvider auth_required recovery', + rationale='Agent-managed methods are automatic; env and terminal setup stays client-owned.', ), }, ) diff --git a/docs/pydantic-acp.md b/docs/pydantic-acp.md index 66be201..4ee12cc 100644 --- a/docs/pydantic-acp.md +++ b/docs/pydantic-acp.md @@ -102,6 +102,7 @@ model = create_acp_model( acp_command=("npx", "@zed-industries/codex-acp"), cwd="/workspace", stderr_mode="inherit", + raise_on_empty_turn=True, ) agent = Agent(model) ``` @@ -112,6 +113,37 @@ ACP agent exposes a selectable `"model"` `session/set_config_option` option. `AcpProvider` and `AcpModel` remain available when lower-level provider ownership is needed. +For command-backed and in-process ACP agents, session setup is explicit and +does not require a dummy prompt: + +```python +from acp.interfaces import Agent as AcpAgent +from pydantic_acp import AcpProvider + + +async def prepare_provider(acp_agent: AcpAgent) -> AcpProvider: + provider = AcpProvider( + acp_agent=acp_agent, + cwd="/workspace", + raise_on_empty_turn=True, + ) + await provider.ensure_session() + await provider.set_session_mode("review") + return provider +``` + +If `session/new` fails with ACP `auth_required`, `AcpProvider` calls +`authenticate` with the first advertised `AuthMethodAgent` and retries session +creation once. `auth_method_id="..."` selects a specific method when the host +has prepared it. `EnvVarAuthMethod` and `TerminalAuthMethod` require +client-owned credential injection or terminal execution, so the provider does +not select them automatically. + +Agent errors are propagated without single-child anyio TaskGroup wrapping. +`raise_on_empty_turn=True` additionally turns a text request that produces no +visible agent text into `UnexpectedModelBehavior`; the default remains `False` +for backward compatibility. + The bridge keeps ownership explicit: - Pydantic AI owns the outer run, output validation, and normal provider diff --git a/packages/adapters/langchain-acp/VERSION b/packages/adapters/langchain-acp/VERSION index dc1e644..26ca594 100644 --- a/packages/adapters/langchain-acp/VERSION +++ b/packages/adapters/langchain-acp/VERSION @@ -1 +1 @@ -1.6.0 +1.5.1 diff --git a/packages/adapters/langchain-acp/src/langchain_acp/_version.py b/packages/adapters/langchain-acp/src/langchain_acp/_version.py index 7ce04b8..f8a095a 100644 --- a/packages/adapters/langchain-acp/src/langchain_acp/_version.py +++ b/packages/adapters/langchain-acp/src/langchain_acp/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.6.0" +__version__ = "1.5.1" diff --git a/packages/adapters/pydantic-acp/README.md b/packages/adapters/pydantic-acp/README.md index cc1df87..4a91ca8 100644 --- a/packages/adapters/pydantic-acp/README.md +++ b/packages/adapters/pydantic-acp/README.md @@ -121,10 +121,19 @@ model = create_acp_model( acp_command=("npx", "@zed-industries/codex-acp"), cwd="/workspace", stderr_mode="inherit", + raise_on_empty_turn=True, ) agent = Agent(model) ``` +`raise_on_empty_turn=True` converts a silent text turn into an ACP-specific +`UnexpectedModelBehavior` instead of returning an empty response. When +`session/new` reports `auth_required`, the provider calls `authenticate` with +the first advertised agent-managed method and retries session creation once. +Use `auth_method_id="..."` to select a specific method that has already been +prepared by the host. Environment-variable and terminal auth methods require +client-side credential or terminal setup and are not selected automatically. + For lower-level ownership, construct the provider directly: ```python @@ -133,6 +142,8 @@ from pydantic_acp import AcpProvider # `remote_acp_agent` can be any object implementing the ACP Agent interface. provider = AcpProvider(acp_agent=remote_acp_agent, cwd="/workspace") +session_id = await provider.ensure_session() +await provider.set_session_mode("review") model = provider.model() agent = Agent(model) @@ -144,6 +155,7 @@ This keeps ownership boundaries explicit: - Pydantic AI owns the outer agent run, output validation, and normal model/provider lifecycle. - ACP owns the delegated agent session, ACP-visible updates, and any editor or host capabilities requested by that agent. +- `ensure_session()` initializes and creates the ACP session without consuming a prompt turn; `set_session_mode(...)` uses that same session. - `create_acp_model(...)` and `provider.model()` leave ACP model selection to the wrapped agent's session default; pass `model_name="zed-agent"` or `provider.model("zed-agent")` only when the ACP agent exposes a selectable `"model"` `session/set_config_option` option. - `AcpHostBridge` records ACP `session_update` messages and can delegate filesystem, terminal, approval, and extension callbacks to a real ACP host client when one is supplied. - Pydantic AI function tools are intentionally not executed directly by `AcpModel`; register tools on the ACP agent or expose host capabilities through ACP. diff --git a/packages/adapters/pydantic-acp/VERSION b/packages/adapters/pydantic-acp/VERSION index dc1e644..26ca594 100644 --- a/packages/adapters/pydantic-acp/VERSION +++ b/packages/adapters/pydantic-acp/VERSION @@ -1 +1 @@ -1.6.0 +1.5.1 diff --git a/packages/adapters/pydantic-acp/src/pydantic_acp/_version.py b/packages/adapters/pydantic-acp/src/pydantic_acp/_version.py index 7ce04b8..f8a095a 100644 --- a/packages/adapters/pydantic-acp/src/pydantic_acp/_version.py +++ b/packages/adapters/pydantic-acp/src/pydantic_acp/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.6.0" +__version__ = "1.5.1" diff --git a/packages/adapters/pydantic-acp/src/pydantic_acp/client.py b/packages/adapters/pydantic-acp/src/pydantic_acp/client.py index 6eda890..ed1ab91 100644 --- a/packages/adapters/pydantic-acp/src/pydantic_acp/client.py +++ b/packages/adapters/pydantic-acp/src/pydantic_acp/client.py @@ -16,11 +16,13 @@ from acp.interfaces import Client as AcpClient from acp.schema import ( AgentMessageChunk, + AuthMethodAgent, ClientCapabilities, ClientSessionCapabilities, CreateElicitationResponse, CreateTerminalResponse, ElicitationMode, + EnvVarAuthMethod, EnvVariable, Implementation, KillTerminalResponse, @@ -31,6 +33,7 @@ RequestPermissionResponse, SessionConfigOptionsCapabilities, SessionConfigOptionSelect, + TerminalAuthMethod, TerminalOutputResponse, ToolCallUpdate, UsageUpdate, @@ -65,6 +68,7 @@ from pydantic_ai.providers import Provider from pydantic_ai.settings import ModelSettings from pydantic_ai.usage import RequestUsage +from typing_extensions import TypeIs from ._meta_protocol import ( MISSING_STRUCTURED_OUTPUT, @@ -76,7 +80,9 @@ from .types import AgentPromptBlock HistoryMode: TypeAlias = Literal["latest_user", "full"] +AuthMethod: TypeAlias = EnvVarAuthMethod | TerminalAuthMethod | AuthMethodAgent _DEFAULT_MODEL_NAME = "agent" +_TASK_GROUP_ERROR_MESSAGE = "unhandled errors in a TaskGroup" _AUTH_REQUIRED_DIAGNOSTIC = ( "The ACP agent requires authentication (session/new returned auth_required / -32000)" ) @@ -412,7 +418,11 @@ def _default_client_capabilities() -> ClientCapabilities: ) -def _unwrap_acp_error(exc: BaseException) -> BaseException: +def _is_task_group_error(exc: BaseException) -> TypeIs[ExceptionGroup[Exception]]: + return isinstance(exc, ExceptionGroup) and exc.message == _TASK_GROUP_ERROR_MESSAGE + + +def _unwrap_acp_error(exc: Exception) -> Exception: """Return the ACP agent's real error, free of anyio TaskGroup wrapping. ACP calls run over a stdio connection whose background reader lives in an @@ -422,16 +432,17 @@ def _unwrap_acp_error(exc: BaseException) -> BaseException: TaskGroup"), or as the real error carrying that group as its ``__context__``. Both bury the actual cause. This peels single-child groups down to the leaf and drops a TaskGroup ``__context__`` so the meaningful - error propagates on its own instead of as an opaque group. + error propagates on its own instead of as an opaque group. Other exception + groups are preserved because they may carry meaningful aggregate failures. """ - leaf = exc - seen: set[int] = set() + leaf: Exception = exc while ( - isinstance(leaf, BaseExceptionGroup) and len(leaf.exceptions) == 1 and id(leaf) not in seen + _is_task_group_error(leaf) + and len(leaf.exceptions) == 1 + and isinstance(leaf.exceptions[0], Exception) ): - seen.add(id(leaf)) leaf = leaf.exceptions[0] - if isinstance(leaf.__context__, BaseExceptionGroup): + if leaf.__context__ is not None and _is_task_group_error(leaf.__context__): leaf.__context__ = None leaf.__suppress_context__ = True return leaf @@ -548,7 +559,7 @@ def __init__( self._session_id: str | None = None self._current_model_name: str | None = None self._model_config_option_available: bool | None = None - self._auth_methods: list[Any] = [] + self._auth_methods: list[AuthMethod] = [] self._authenticated = False self._session_lock: asyncio.Lock | None = None self._session_lock_loop: asyncio.AbstractEventLoop | None = None @@ -715,7 +726,7 @@ async def request_prompt( ) except asyncio.CancelledError: raise - except BaseException as exc: + except Exception as exc: cleaned = _unwrap_acp_error(exc) if cleaned is exc: raise @@ -800,7 +811,7 @@ async def _new_session_with_auth(self) -> NewSessionResponse: await self._authenticate() return await self._call_new_session() - async def _call_new_session(self) -> Any: + async def _call_new_session(self) -> NewSessionResponse: return await self._client.new_session( cwd=self._cwd, mcp_servers=list(self._mcp_servers), @@ -814,19 +825,18 @@ async def _authenticate(self) -> None: f"{_AUTH_REQUIRED_DIAGNOSTIC} but the wrapped agent does not expose an " "'authenticate' method, so the session cannot be established." ) - method_id = self._auth_method_id or next( - (getattr(method, "id", None) for method in self._auth_methods), + agent_method = next( + (method for method in self._auth_methods if isinstance(method, AuthMethodAgent)), None, ) + method_id = self._auth_method_id or (agent_method.id if agent_method is not None else None) if method_id is None: - advertised = ", ".join( - str(getattr(method, "id", method)) for method in self._auth_methods - ) + advertised = ", ".join(method.id for method in self._auth_methods) raise UserError( - f"{_AUTH_REQUIRED_DIAGNOSTIC} but advertised no usable authentication " + f"{_AUTH_REQUIRED_DIAGNOSTIC} but advertised no agent-managed authentication " f"method to satisfy it{f' (advertised: {advertised})' if advertised else ''}. " - "Pass auth_method_id=... to AcpProvider, or authenticate the agent " - "out of band." + "Environment-variable and terminal methods require client-side setup. Complete " + "that setup out of band, or pass auth_method_id=... after preparing the agent." ) result = authenticate(method_id=method_id) if inspect.isawaitable(result): diff --git a/packages/adapters/pydantic-acp/src/pydantic_acp/factory.py b/packages/adapters/pydantic-acp/src/pydantic_acp/factory.py index 1eee11b..77a0e5a 100644 --- a/packages/adapters/pydantic-acp/src/pydantic_acp/factory.py +++ b/packages/adapters/pydantic-acp/src/pydantic_acp/factory.py @@ -27,6 +27,8 @@ def create_acp_model( history_mode: HistoryMode = "latest_user", delegate_client: AcpClient | None = None, enable_pydantic_acp_meta: bool | None = None, + auth_method_id: str | None = None, + raise_on_empty_turn: bool = False, settings: ModelSettings | None = None, profile: ModelProfileSpec | None = None, ) -> AcpModel: @@ -35,6 +37,10 @@ def create_acp_model( Exactly one of ``acp_agent`` or ``acp_command`` must be provided. Passing ``model_name=None`` leaves ACP model selection to the remote agent's session default and does not send a ``session/set_config_option`` request for ``"model"``. + ``auth_method_id`` selects an explicit ACP authentication method when + ``session/new`` reports ``auth_required``. Set ``raise_on_empty_turn=True`` + when a silent ACP turn should fail as ``UnexpectedModelBehavior`` instead + of producing an empty Pydantic AI response. """ command = _normalize_command(acp_command) @@ -63,6 +69,8 @@ def create_acp_model( prompt_renderer=prompt_renderer, history_mode=history_mode, enable_pydantic_acp_meta=enable_pydantic_acp_meta, + auth_method_id=auth_method_id, + raise_on_empty_turn=raise_on_empty_turn, ) return provider.model( model_name, diff --git a/packages/helpers/codex-auth-helper/VERSION b/packages/helpers/codex-auth-helper/VERSION index dc1e644..26ca594 100644 --- a/packages/helpers/codex-auth-helper/VERSION +++ b/packages/helpers/codex-auth-helper/VERSION @@ -1 +1 @@ -1.6.0 +1.5.1 diff --git a/packages/helpers/codex-auth-helper/src/codex_auth_helper/_version.py b/packages/helpers/codex-auth-helper/src/codex_auth_helper/_version.py index 7ce04b8..f8a095a 100644 --- a/packages/helpers/codex-auth-helper/src/codex_auth_helper/_version.py +++ b/packages/helpers/codex-auth-helper/src/codex_auth_helper/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.6.0" +__version__ = "1.5.1" diff --git a/packages/transports/acpremote/VERSION b/packages/transports/acpremote/VERSION index dc1e644..26ca594 100644 --- a/packages/transports/acpremote/VERSION +++ b/packages/transports/acpremote/VERSION @@ -1 +1 @@ -1.6.0 +1.5.1 diff --git a/packages/transports/acpremote/src/acpremote/_version.py b/packages/transports/acpremote/src/acpremote/_version.py index 7ce04b8..f8a095a 100644 --- a/packages/transports/acpremote/src/acpremote/_version.py +++ b/packages/transports/acpremote/src/acpremote/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.6.0" +__version__ = "1.5.1" diff --git a/pyproject.toml b/pyproject.toml index 701387f..2db0b3d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,12 +23,12 @@ dependencies = [ ] [project.optional-dependencies] -codex = ["codex-auth-helper>=1.6.0,<2.0.0"] -deepagents = ["langchain-acp[deepagents]>=1.6.0,<2.0.0"] -langchain = ["langchain-acp>=1.6.0,<2.0.0"] +codex = ["codex-auth-helper>=1.5.1,<2.0.0"] +deepagents = ["langchain-acp[deepagents]>=1.5.1,<2.0.0"] +langchain = ["langchain-acp>=1.5.1,<2.0.0"] launch = ["uv>=0.8.3"] -pydantic = ["pydantic-acp>=1.6.0,<2.0.0"] -remote = ["acpremote>=1.6.0,<2.0.0"] +pydantic = ["pydantic-acp>=1.5.1,<2.0.0"] +remote = ["acpremote>=1.5.1,<2.0.0"] dev = [ "acpremote", diff --git a/src/acpkit/_version.py b/src/acpkit/_version.py index 7ce04b8..f8a095a 100644 --- a/src/acpkit/_version.py +++ b/src/acpkit/_version.py @@ -2,4 +2,4 @@ __all__ = ("__version__",) -__version__ = "1.6.0" +__version__ = "1.5.1" diff --git a/tests/pydantic/test_acp_client_provider.py b/tests/pydantic/test_acp_client_provider.py index 0541914..aa3ab41 100644 --- a/tests/pydantic/test_acp_client_provider.py +++ b/tests/pydantic/test_acp_client_provider.py @@ -6,7 +6,7 @@ import tomllib from pathlib import Path from types import SimpleNamespace -from typing import Any, Literal, cast +from typing import Any, Literal, TypeAlias, cast from unittest.mock import AsyncMock import pydantic_acp @@ -22,11 +22,13 @@ AgentCapabilities, AgentMessageChunk, AllowedOutcome, + AuthEnvVar, AuthMethodAgent, ClientCapabilities, ElicitationFormSessionMode, ElicitationMode, ElicitationSchema, + EnvVarAuthMethod, Implementation, InitializeResponse, NewSessionResponse, @@ -35,6 +37,7 @@ SessionConfigOptionSelect, SessionConfigSelectOption, SetSessionConfigOptionResponse, + TerminalAuthMethod, ToolCallUpdate, Usage, UsageUpdate, @@ -77,6 +80,8 @@ from .support import HostRecordingClient, RecordingClient +TestAuthMethod: TypeAlias = EnvVarAuthMethod | TerminalAuthMethod | AuthMethodAgent + class EchoACPAgent: # type: ignore[misc] def __init__( @@ -416,6 +421,33 @@ async def test_create_acp_model_wraps_in_process_acp_agent() -> None: assert acp_agent.session_models == [("session-1", "zed-agent")] +async def test_create_acp_model_forwards_auth_and_empty_turn_options() -> None: + auth_agent = AuthRequiredACPAgent( + auth_methods=[AuthMethodAgent(id="login", name="Login")], + ) + auth_model = create_acp_model( + acp_agent=cast(AcpAgent, auth_agent), + cwd="/workspace", + auth_method_id="custom", + ) + + await cast(AcpProvider, auth_model.provider).ensure_session() + + assert auth_agent.authenticated_with == ["custom"] + + empty_model = create_acp_model( + acp_agent=cast(AcpAgent, NoHandshakeACPAgent()), + cwd="/workspace", + raise_on_empty_turn=True, + ) + with pytest.raises(UnexpectedModelBehavior, match="ACP agent ended its turn"): + await empty_model.request( + [ModelRequest(parts=[UserPromptPart("hello")])], + None, + ModelRequestParameters(), + ) + + async def test_acp_provider_default_model_leaves_remote_model_selection_to_agent() -> None: acp_agent = EchoACPAgent() set_session_model = AsyncMock( @@ -2173,7 +2205,7 @@ def test_acp_model_profile_disables_tool_and_structured_output_support() -> None class AuthRequiredACPAgent(EchoACPAgent): """An ACP agent that rejects ``session/new`` until ``authenticate`` runs.""" - def __init__(self, *, auth_methods: list[Any], fail_times: int = 1) -> None: + def __init__(self, *, auth_methods: list[TestAuthMethod], fail_times: int = 1) -> None: super().__init__() self._auth_methods = auth_methods self._fail_times = fail_times @@ -2385,6 +2417,25 @@ async def test_acp_provider_uses_first_advertised_auth_method_when_multiple_exis assert agent.authenticated_with == ["oauth"] +async def test_acp_provider_automatically_uses_only_agent_managed_auth_methods() -> None: + agent = AuthRequiredACPAgent( + auth_methods=[ + EnvVarAuthMethod( + id="api-key", + name="API key", + type="env_var", + vars=[AuthEnvVar(name="API_KEY")], + ), + AuthMethodAgent(id="oauth", name="OAuth", description="Browser login"), + ], + ) + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") + + await provider.ensure_session() + + assert agent.authenticated_with == ["oauth"] + + async def test_acp_provider_supports_synchronous_authenticate() -> None: # A *synchronous* authenticate() exercises the non-awaitable branch of # AcpProvider._authenticate. It subclasses EchoACPAgent (which has no @@ -2449,38 +2500,24 @@ async def test_acp_provider_auth_required_without_any_method_error_omits_adverti assert "(advertised:" not in str(exc_info.value) -async def test_acp_provider_auth_required_with_unusable_advertised_methods_raises_userror() -> None: - class AuthMethodsWithoutIdACPAgent(EchoACPAgent): - async def initialize( - self, - protocol_version: int, - client_capabilities: ClientCapabilities | None = None, - client_info: Implementation | None = None, - **kwargs: Any, - ) -> Any: - del client_capabilities, client_info, kwargs - self.initialized_protocols.append(protocol_version) - return SimpleNamespace(auth_methods=[SimpleNamespace(name="mystery")]) - - async def new_session( - self, - cwd: str, - mcp_servers: list[Any] | None = None, - **kwargs: Any, - ) -> NewSessionResponse: - del cwd, mcp_servers, kwargs - raise RequestError.auth_required() - - async def authenticate(self, method_id: str, **kwargs: Any) -> None: - del method_id, kwargs - - provider = AcpProvider( - acp_agent=cast(AcpAgent, AuthMethodsWithoutIdACPAgent()), cwd="/workspace" +async def test_acp_provider_requires_client_setup_for_env_var_auth_method() -> None: + agent = AuthRequiredACPAgent( + auth_methods=[ + EnvVarAuthMethod( + id="api-key", + name="API key", + type="env_var", + vars=[AuthEnvVar(name="API_KEY")], + ) + ], ) + provider = AcpProvider(acp_agent=cast(AcpAgent, agent), cwd="/workspace") - with pytest.raises(UserError, match="advertised no usable authentication"): + with pytest.raises(UserError, match="require client-side setup"): await provider.ensure_session() + assert agent.authenticated_with == [] + async def test_acp_provider_ensure_session_forwards_model_name() -> None: agent = EchoACPAgent() @@ -2582,19 +2619,22 @@ async def prompt( def test_unwrap_acp_error_peels_single_child_group() -> None: inner = RequestError(-32000, "Rate limited") - group = BaseExceptionGroup("unhandled errors in a TaskGroup", [inner]) + group = ExceptionGroup("unhandled errors in a TaskGroup", [inner]) assert client_module._unwrap_acp_error(group) is inner def test_unwrap_acp_error_peels_nested_single_child_groups() -> None: inner = RequestError(-32000, "Rate limited") - nested = BaseExceptionGroup("outer", [BaseExceptionGroup("inner", [inner])]) + nested = ExceptionGroup( + "unhandled errors in a TaskGroup", + [ExceptionGroup("unhandled errors in a TaskGroup", [inner])], + ) assert client_module._unwrap_acp_error(nested) is inner def test_unwrap_acp_error_drops_taskgroup_context_noise() -> None: err = RuntimeError("boom") - err.__context__ = BaseExceptionGroup("unhandled errors in a TaskGroup", [ValueError("x")]) + err.__context__ = ExceptionGroup("unhandled errors in a TaskGroup", [ValueError("x")]) cleaned = client_module._unwrap_acp_error(err) assert cleaned is err assert cleaned.__context__ is None @@ -2607,8 +2647,18 @@ def test_unwrap_acp_error_leaves_plain_exception_untouched() -> None: def test_unwrap_acp_error_keeps_multi_child_group() -> None: - group = BaseExceptionGroup("many", [ValueError("a"), RequestError(-32000, "b")]) + group = ExceptionGroup("many", [ValueError("a"), RequestError(-32000, "b")]) + assert client_module._unwrap_acp_error(group) is group + + +def test_unwrap_acp_error_preserves_unrelated_single_child_group_and_context() -> None: + context = ExceptionGroup("validation failures", [ValueError("context")]) + inner = RuntimeError("boom") + inner.__context__ = context + group = ExceptionGroup("validation failures", [inner]) + assert client_module._unwrap_acp_error(group) is group + assert inner.__context__ is context async def test_request_prompt_unwraps_taskgroup_error_from_the_agent() -> None: @@ -2621,7 +2671,7 @@ async def prompt( **kwargs: Any, ) -> PromptResponse: del prompt, session_id, message_id, kwargs - raise BaseExceptionGroup( + raise ExceptionGroup( "unhandled errors in a TaskGroup", [RequestError(-32000, "Rate limited")], )