diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 61a76cf4d..dc3dbf62f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -257,6 +257,7 @@ jobs: from band import Agent, BandLink, AgentRuntime from band.config import load_agent_config from band_sdk_core import ( + AgentFailure, ClaimRegistry, ParticipantRoster, RetryTracker, @@ -291,6 +292,13 @@ jobs: "room_id": "room-1", "message_id": "msg-1", } + failure = AgentFailure("wheel-smoke", "failure") + assert failure.to_dict() == { + "provider": "wheel-smoke", + "code": None, + "message": "failure", + "detail": None, + } print('Core imports successful') PYEOF diff --git a/docs/adapters/codex.md b/docs/adapters/codex.md index 0b02d57ab..92af1f8fe 100644 --- a/docs/adapters/codex.md +++ b/docs/adapters/codex.md @@ -249,7 +249,6 @@ These `CodexAdapterConfig(...)` flags add more telemetry detail: | `emit_turn_lifecycle_events` | `bool` | `False` | Emit enriched turn lifecycle events at turn start and completion. | | `emit_diff_events` | `bool` | `False` | Include file diffs in event metadata, capped at 64 KB. | | `emit_token_usage_events` | `bool` | `False` | Track and emit token usage per session. | -| `structured_errors` | `bool` | `True` | Emit structured error events instead of plain text errors. | Enabling both `emit_turn_task_markers` and `emit_turn_lifecycle_events` produces two task events per completed turn. Pick one; lifecycle events contain richer metadata. diff --git a/pyproject.toml b/pyproject.toml index 8d360d50e..4f5dd876d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,7 +23,7 @@ classifiers = [ dependencies = [ "band-client-rest==0.0.27", - "band-sdk-core==2.2.0", + "band-sdk-core==2.3.0", "phoenix-channels-python-client>=0.2.4", "python-dotenv>=1.2.2", "pydantic>=2.0", diff --git a/src/band/adapters/agno.py b/src/band/adapters/agno.py index 5a85f0c7f..0f1bc531a 100644 --- a/src/band/adapters/agno.py +++ b/src/band/adapters/agno.py @@ -12,9 +12,10 @@ from agno.media import Image from agno.tools.function import ToolResult +from band_sdk_core import AgentFailure from typing_extensions import Unpack -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE, AgentToolsProtocol from band.core.simple_adapter import SimpleAdapter from band.core.tool_filter import filter_tool_schemas from band.core.types import ( @@ -465,18 +466,18 @@ async def _run_agent( :meth:`_run_streamed`), matching the other adapters' live reporting. Otherwise it runs non-streaming, exactly as before. """ - agent = self._agent - if agent is None: - raise RuntimeError("AgnoAdapter was used before on_started()") - session_id = self._session_id_factory(room_id) - logger.debug( - "Room %s msg %s: running Agno agent (%d input messages, session_id=%s)", - room_id, - msg_id, - len(messages), - session_id, - ) try: + agent = self._agent + if agent is None: + raise RuntimeError("AgnoAdapter was used before on_started()") + session_id = self._session_id_factory(room_id) + logger.debug( + "Room %s msg %s: running Agno agent (%d input messages, session_id=%s)", + room_id, + msg_id, + len(messages), + session_id, + ) with _bind_room_tools(tools): if Emit.TOOL_CALLS in self.features.emit: response = await self._run_streamed( @@ -494,22 +495,19 @@ async def _run_agent( # the turn as failed rather than as a silent empty reply. if response is not None and response.status == RunStatus.error: raise AgnoRunError(_error_summary(response.content)) - except Exception: + except Exception as e: # Keep the user-facing payload generic; the full traceback is in the # agent log via logger.exception. Exception text can include DB - # strings, paths, and tokens that must not surface in chat. + # strings, paths, and tokens that must not surface in chat. Only + # the coarse RunStatus.error code -- never response.content -- is + # safe to attach. logger.exception( "Room %s msg %s: error running Agno agent", room_id, msg_id ) - try: - await tools.send_event( - content="Internal error while processing message; see agent logs.", - message_type="error", - ) - except Exception: - logger.exception( - "Room %s msg %s: failed to report error event", room_id, msg_id - ) + code = RunStatus.error.value if isinstance(e, AgnoRunError) else None + await tools.send_failure( + AgentFailure("agno", GENERIC_PROVIDER_FAILURE_MESSAGE, code) + ) raise if response is None: diff --git a/src/band/adapters/anthropic.py b/src/band/adapters/anthropic.py index 5560672c7..25dc557a6 100644 --- a/src/band/adapters/anthropic.py +++ b/src/band/adapters/anthropic.py @@ -11,12 +11,13 @@ import warnings from typing import Any, ClassVar, cast -from anthropic import AsyncAnthropic +from anthropic import APIStatusError, AsyncAnthropic from anthropic.types import Message, MessageParam, TextBlock, ToolParam, ToolUseBlock +from band_sdk_core import AgentFailure from typing_extensions import Unpack from band.core.exceptions import BandConfigError -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE, AgentToolsProtocol from band.core.simple_adapter import SimpleAdapter from band.core.types import ( Capability, @@ -62,6 +63,17 @@ def _image_tool_result_content(result: dict[str, Any]) -> list[dict[str, Any]]: ] +def _to_agent_failure(e: Exception) -> AgentFailure: + """Parse a turn-ending exception into the shared provider-failure shape. + + ``APIStatusError`` carries an HTTP status and response body that a plain + exception's message alone does not. + """ + if isinstance(e, APIStatusError): + return AgentFailure("anthropic", str(e), str(e.status_code), e.body) + return AgentFailure("anthropic", GENERIC_PROVIDER_FAILURE_MESSAGE) + + class AnthropicAdapter(SimpleAdapter[AnthropicMessages]): """ Anthropic SDK adapter using SimpleAdapter pattern. @@ -279,7 +291,7 @@ async def on_message( ) except Exception as e: logger.error("Error calling Anthropic: %s", e, exc_info=True) - await self._report_error(tools, str(e)) + await tools.send_failure(_to_agent_failure(e)) raise # Re-raise so message is marked as failed turn_usage = turn_usage + self._usage_from_response(response) @@ -512,11 +524,3 @@ async def _process_tool_calls( ) return tool_results - - # --- Copied from BaseFrameworkAgent._report_error --- - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - """Send error event (best effort).""" - try: - await tools.send_event(content=f"Error: {error}", message_type="error") - except Exception as e: - logger.warning("Failed to send error event: %s", e) diff --git a/src/band/adapters/claude_sdk.py b/src/band/adapters/claude_sdk.py index 14a1af986..e3c0c4d9a 100644 --- a/src/band/adapters/claude_sdk.py +++ b/src/band/adapters/claude_sdk.py @@ -49,9 +49,14 @@ except ImportError: _CLAUDE_SDK_AVAILABLE = False +from band_sdk_core import AgentFailure from typing_extensions import Unpack -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, + TurnResultAlreadyReported, +) from band.core.simple_adapter import SimpleAdapter from band.core.types import ( Capability, @@ -128,6 +133,8 @@ # same constant instead of a second, driftable number. _CLAUDE_SDK_MAX_BUFFER_BYTES = MAX_INLINE_IMAGE_BYTES * 2 +_PROVIDER = "claude_sdk" + # Approval flow types (mirrors Codex adapter patterns) ApprovalMode = Literal["auto_accept", "auto_decline", "manual"] ApprovalDecision = Literal["accept", "decline"] @@ -611,10 +618,27 @@ async def on_message( stored_session_id, resume_exc, ) - client = await self._session_manager.get_or_create_session( - room_id, resume_session_id=None - ) + try: + client = await self._session_manager.get_or_create_session( + room_id, resume_session_id=None + ) + except Exception as fresh_exc: + logger.exception( + "Room %s: Fresh session creation also failed: %s", + room_id, + fresh_exc, + ) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) + raise else: + logger.exception( + "Room %s: Session creation failed: %s", room_id, resume_exc + ) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) raise # Add chat_id context (Claude needs this for tool calls) -- the label @@ -685,6 +709,11 @@ async def on_message( # Process streaming response (MCP tools handle execution) await self._process_response(client, room_id, tools) + except TurnResultAlreadyReported: + # _on_turn_complete already reported this failure via + # send_failure; propagate without reporting it a second time. + raise + except CLIConnectionError as e: # CLI process is dead — evict the cached session so the next # message creates a fresh one instead of reusing the corpse. @@ -695,12 +724,16 @@ async def on_message( ) await self._invalidate_session(room_id) - await self._report_error(tools, str(e)) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) raise except Exception as e: logger.exception("Error processing message: %s", e) - await self._report_error(tools, str(e)) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) raise logger.debug("Message %s processed successfully", msg.id) @@ -933,11 +966,22 @@ async def _on_turn_complete( # outright) doesn't linger and grow this room's entry unbounded. notified = self._notified_declines.pop(room_id, None) if sdk_message.is_error: - await self._report_error(tools, self._result_error_detail(sdk_message)) + code = ( + str(sdk_message.api_error_status) + if sdk_message.api_error_status is not None + else None + ) + detail = self._result_error_detail(sdk_message) + await tools.send_failure( + AgentFailure(_PROVIDER, detail, code, sdk_message.errors) + ) + raise TurnResultAlreadyReported(detail) elif not replied_this_turn and not self._declined_the_reply( sdk_message.permission_denials, notified ): - await self._report_error(tools, missing_reply_error("Claude SDK")) + detail = missing_reply_error("Claude SDK") + await tools.send_failure(AgentFailure(_PROVIDER, detail)) + raise TurnResultAlreadyReported(detail) def _declined_the_reply( self, permission_denials: list[Any] | None, notified: set[str] | None @@ -1127,14 +1171,6 @@ async def on_cleanup(self, room_id: str) -> None: self._pending_tool_names.pop(room_id, None) logger.debug("Room %s: Cleaned up Claude SDK session", room_id) - # --- Copied from BaseFrameworkAgent._report_error --- - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - """Send error event (best effort).""" - try: - await tools.send_event(content=f"Error: {error}", message_type="error") - except Exception: - logger.debug("Failed to send error event", exc_info=True) - async def cleanup_all(self) -> None: """Cleanup all sessions (call on stop).""" # Decline all pending approvals across rooms diff --git a/src/band/adapters/codex.py b/src/band/adapters/codex.py index 9ad53f155..0072a2c0d 100644 --- a/src/band/adapters/codex.py +++ b/src/band/adapters/codex.py @@ -12,13 +12,25 @@ from datetime import datetime, timezone from typing import ClassVar, Any, Callable, Literal, NamedTuple, Protocol +from band_sdk_core import AgentFailure from pydantic import AliasChoices, BaseModel, Field, ValidationError, field_validator from pydantic_settings import BaseSettings, SettingsConfigDict from typing_extensions import Unpack from band.converters.codex import CodexHistoryConverter from band.converters.helpers import build_replay_messages -from band.core.protocols import AgentToolsProtocol +from band.core.delivery import ( + DeliveryFailedError, + deliver_reply, + reraise_delivery_cause, +) +from band.core.protocols import ( + FAILURE_CODE_TIMEOUT, + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, + TurnResultAlreadyReported, + send_event_safe, +) from band.core.simple_adapter import SimpleAdapter from band.core.types import ( AgentInput, @@ -37,12 +49,13 @@ ) from band.integrations.codex.types import ( CODEX_APPROVAL_METHODS, + CODEX_PROVIDER, ApprovalAuditEntry, CodexApprovalMethod, CodexItemType, CodexSessionState, CodexTokenUsage, - build_structured_error_metadata, + build_agent_failure, parse_plan_steps, ) from band.runtime.custom_tools import ( @@ -356,8 +369,6 @@ class CodexAdapterConfig(BaseSettings): # File-change approvals always key on the sorted set of paths being # modified, independent of this flag. session_approval_granularity: Literal["binary", "full_command"] = "full_command" - # --- Phase 1: Structured errors & enriched approvals --- - structured_errors: bool = True # --- Phase 2: Plan & task lifecycle --- stream_plan_events: bool = False emit_turn_lifecycle_events: bool = False @@ -525,7 +536,7 @@ def _log_startup_config(self, agent_name: str) -> None: "execution_reporting=%s, self_config_tools=%s, " "task_events=%s, turn_markers=%s, thought_events=%s, " "stream_reasoning=%s, stream_plan=%s, stream_commentary=%s, " - "diffs=%s, token_usage=%s, structured_errors=%s", + "diffs=%s, token_usage=%s", agent_name, self.config.transport, self._selected_model or self.config.model or "auto", @@ -541,7 +552,6 @@ def _log_startup_config(self, agent_name: str) -> None: self.config.stream_commentary_events, self.config.emit_diff_events, self.config.emit_token_usage_events, - self.config.structured_errors, ) async def on_event(self, inp: AgentInput) -> None: @@ -571,88 +581,99 @@ async def on_message( "decline", "approvals", }: - handled = await self._handle_approval_command( - tools=tools, - msg=msg, - room_id=room_id, - command=command[0], - args=command[1], - ) - if handled: - return - - async with self._rpc_lock: - await self._ensure_client_ready() - if self._client is None: - raise RuntimeError( - "Codex client not initialized after _ensure_client_ready" - ) - - if command is not None: - handled = await self._handle_local_command( + try: + handled = await self._handle_approval_command( tools=tools, msg=msg, - history=history, room_id=room_id, command=command[0], args=command[1], ) - if handled: - return + except DeliveryFailedError as e: + reraise_delivery_cause(e) + if handled: + return - thread_id = await self._ensure_thread( - room_id=room_id, - history=history, - tools=tools, - is_session_bootstrap=is_session_bootstrap, - ) + async with self._rpc_lock: + thread_id: str | None = None + turn_id: str | None = None + try: + await self._ensure_client_ready() + if self._client is None: + raise RuntimeError( + "Codex client not initialized after _ensure_client_ready" + ) - turn_input, has_pending_prompt_injection = self._build_turn_input( - msg=msg, - participants_msg=participants_msg, - contacts_msg=contacts_msg, - room_id=room_id, - ) + if command is not None: + handled = await self._handle_local_command( + tools=tools, + msg=msg, + history=history, + room_id=room_id, + command=command[0], + args=command[1], + ) + if handled: + return - turn_params: dict[str, Any] = { - "threadId": thread_id, - "input": turn_input, - } - self._apply_turn_overrides(turn_params, room_id=room_id) + thread_id = await self._ensure_thread( + room_id=room_id, + history=history, + tools=tools, + is_session_bootstrap=is_session_bootstrap, + ) - turn_started = await self._start_turn(turn_params) - if has_pending_prompt_injection: - self._prompt_injected_rooms.add(room_id) - turn = turn_started.get("turn") if isinstance(turn_started, dict) else {} - turn_id = str((turn or {}).get("id") or "") + turn_input, has_pending_prompt_injection = self._build_turn_input( + msg=msg, + participants_msg=participants_msg, + contacts_msg=contacts_msg, + room_id=room_id, + ) - if ( - Emit.TASK_EVENTS in self.features.emit - and self.config.emit_turn_task_markers - ): - await tools.send_event( - content=self._build_task_event_content( - task_id=turn_id or None, - task="Codex turn", - status="started", - summary=f"Thread: {thread_id}", - ), - message_type="task", - metadata={ - "codex_thread_id": thread_id, - "codex_turn_id": turn_id or None, - "codex_room_id": room_id, - }, + turn_params: dict[str, Any] = { + "threadId": thread_id, + "input": turn_input, + } + self._apply_turn_overrides(turn_params, room_id=room_id) + + turn_started = await self._start_turn(turn_params) + if has_pending_prompt_injection: + self._prompt_injected_rooms.add(room_id) + turn = ( + turn_started.get("turn") if isinstance(turn_started, dict) else {} ) + turn_id = str((turn or {}).get("id") or "") + + if ( + Emit.TASK_EVENTS in self.features.emit + and self.config.emit_turn_task_markers + ): + await send_event_safe( + tools, + content=self._build_task_event_content( + task_id=turn_id or None, + task="Codex turn", + status="started", + summary=f"Thread: {thread_id}", + ), + message_type="task", + metadata={ + "codex_thread_id": thread_id, + "codex_turn_id": turn_id or None, + "codex_room_id": room_id, + }, + log_label="turn started task event", + log_level=logging.DEBUG, + ) - # Phase 2: Turn STARTED lifecycle event with input summary - if ( - self.config.emit_turn_lifecycle_events - and Emit.TASK_EVENTS in self.features.emit - ): - input_summary = (msg.content or "")[:200] - try: - await tools.send_event( + # Phase 2: Turn STARTED lifecycle event with input summary + if ( + self.config.emit_turn_lifecycle_events + and Emit.TASK_EVENTS in self.features.emit + ): + input_summary = (msg.content or "")[:200] + await send_event_safe( + tools, content=self._build_task_event_content( task_id=turn_id or None, task="Codex turn lifecycle", @@ -668,56 +689,135 @@ async def on_message( "codex_turn_status": "started", "codex_input_summary": input_summary, }, + log_label="turn started lifecycle event", + log_level=logging.DEBUG, + ) + + # Reset per-turn token deltas for the new turn. + usage_obj = self._token_usage.get(thread_id) + if usage_obj is not None: + usage_obj.reset_turn_deltas() + + # perf_counter (not monotonic): highest-resolution clock, so a fast + # turn still measures a non-zero duration on Windows, where + # monotonic()'s coarse tick can round an instant turn to 0.0. + _turn_start = _time.perf_counter() + try: + result = await self._process_turn_events( + tools=tools, + msg=msg, + room_id=room_id, + thread_id=thread_id, + turn_id=turn_id or None, + turn_start=_turn_start, + ) + except TurnResultAlreadyReported: + # A nested handler (e.g. the turn-timeout branch) already + # reported this failure via send_failure; propagate it + # without emitting a friendly _emit_turn_outcome reply or + # a generic "Internal error" fallback. + raise + except CodexJsonRpcError as error: + result = TurnResult( + turn_status="failed", + turn_error=str(error), + ) + await self._emit_failed_turn_outcome( + tools=tools, + msg=msg, + room_id=room_id, + thread_id=thread_id, + turn_id=turn_id or None, + result=result, + turn_start=_turn_start, ) + raise except Exception: - logger.debug( - "Failed to emit turn started lifecycle event", - exc_info=True, + logger.exception( + "Unexpected error during Codex turn event processing " + "(thread=%s, turn=%s)", + thread_id, + turn_id, ) + result = TurnResult( + turn_status="failed", + turn_error="Internal error during turn processing", + ) + await tools.send_failure( + AgentFailure(CODEX_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) + await self._emit_failed_turn_outcome( + tools=tools, + msg=msg, + room_id=room_id, + thread_id=thread_id, + turn_id=turn_id or None, + result=result, + turn_start=_turn_start, + ) + raise TurnResultAlreadyReported( + "Internal error during turn processing" + ) from None - # Reset per-turn token deltas for the new turn. - usage_obj = self._token_usage.get(thread_id) - if usage_obj is not None: - usage_obj.reset_turn_deltas() - - # perf_counter (not monotonic): highest-resolution clock, so a fast - # turn still measures a non-zero duration on Windows, where - # monotonic()'s coarse tick can round an instant turn to 0.0. - _turn_start = _time.perf_counter() - try: - result = await self._process_turn_events( + _turn_duration_s = _time.perf_counter() - _turn_start + await self._emit_turn_outcome( tools=tools, msg=msg, room_id=room_id, thread_id=thread_id, turn_id=turn_id or None, - turn_start=_turn_start, + turn_status=result.turn_status, + turn_error=result.turn_error, + final_text=result.final_text, + saw_send_message_tool=result.saw_send_message_tool, + duration_s=_turn_duration_s, ) + except DeliveryFailedError as e: + reraise_delivery_cause(e) + except TurnResultAlreadyReported: + raise + except CodexJsonRpcError as e: + # A structured RPC error from the app-server (e.g. "model not + # available") is safe, curated text -- unlike an arbitrary + # caught exception, it's worth showing verbatim. + await tools.send_failure(AgentFailure(CODEX_PROVIDER, str(e))) + raise except Exception: logger.exception( - "Unexpected error during Codex turn event processing " - "(thread=%s, turn=%s)", + "Unexpected error in Codex on_message (thread=%s, turn=%s)", thread_id, turn_id, ) - result = TurnResult( - turn_status="failed", - turn_error="Internal error during turn processing", + await tools.send_failure( + AgentFailure(CODEX_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) ) + raise - _turn_duration_s = _time.perf_counter() - _turn_start - await self._emit_turn_outcome( - tools=tools, - msg=msg, - room_id=room_id, - thread_id=thread_id, - turn_id=turn_id or None, - turn_status=result.turn_status, - turn_error=result.turn_error, - final_text=result.final_text, - saw_send_message_tool=result.saw_send_message_tool, - duration_s=_turn_duration_s, - ) + async def _emit_failed_turn_outcome( + self, + *, + tools: AgentToolsProtocol, + msg: PlatformMessage, + room_id: str, + thread_id: str, + turn_id: str | None, + result: TurnResult, + turn_start: float, + ) -> None: + """Emit failure lifecycle events without posting a second reply.""" + await self._emit_turn_outcome( + tools=tools, + msg=msg, + room_id=room_id, + thread_id=thread_id, + turn_id=turn_id, + turn_status=result.turn_status, + turn_error=result.turn_error, + final_text=result.final_text, + saw_send_message_tool=result.saw_send_message_tool, + duration_s=_time.perf_counter() - turn_start, + include_reply=False, + ) async def _process_turn_events( self, @@ -734,6 +834,7 @@ async def _process_turn_events( raise RuntimeError("CodexAdapter client is None during turn event loop") result = TurnResult() + failure_reported = False try: while True: _remaining = max( @@ -770,13 +871,14 @@ async def _process_turn_events( continue if event.method == "error": - await self._handle_error_event( + reported = await self._handle_error_event( tools=tools, params=params, room_id=room_id, thread_id=thread_id, turn_id=turn_id, ) + failure_reported = failure_reported or reported continue # --- Phase 3: Real-time streaming --- @@ -787,45 +889,42 @@ async def _process_turn_events( if self.config.stream_reasoning_events: delta = params.get("delta", "") item_id = str(params.get("itemId") or "") - try: - await tools.send_event( - content=str(delta), - message_type="thought", - metadata={ - "streaming": True, - "codex_item_id": item_id, - "codex_event_type": event.method, - "codex_room_id": room_id, - "codex_thread_id": thread_id, - "codex_turn_id": turn_id, - }, - ) - except Exception: - logger.debug( - "Failed to stream reasoning delta", - exc_info=True, - ) + await send_event_safe( + tools, + content=str(delta), + message_type="thought", + metadata={ + "streaming": True, + "codex_item_id": item_id, + "codex_event_type": event.method, + "codex_room_id": room_id, + "codex_thread_id": thread_id, + "codex_turn_id": turn_id, + }, + log_label="reasoning delta", + log_level=logging.DEBUG, + ) continue if event.method == "item/plan/delta": if self.config.stream_plan_events: delta = params.get("delta", "") item_id = str(params.get("itemId") or "") - try: - await tools.send_event( - content=str(delta), - message_type="thought", - metadata={ - "streaming": True, - "subtype": "plan", - "codex_item_id": item_id, - "codex_room_id": room_id, - "codex_thread_id": thread_id, - "codex_turn_id": turn_id, - }, - ) - except Exception: - logger.debug("Failed to stream plan delta", exc_info=True) + await send_event_safe( + tools, + content=str(delta), + message_type="thought", + metadata={ + "streaming": True, + "subtype": "plan", + "codex_item_id": item_id, + "codex_room_id": room_id, + "codex_thread_id": thread_id, + "codex_turn_id": turn_id, + }, + log_label="plan delta", + log_level=logging.DEBUG, + ) continue # --- Phase 2: Plan step tracking --- @@ -859,27 +958,24 @@ async def _process_turn_events( ): compacted_thread = str(params.get("threadId") or thread_id) compacted_turn = str(params.get("turnId") or turn_id or "") - try: - await tools.send_event( - content=self._build_task_event_content( - task_id=compacted_turn or None, - task="Codex context compaction", - status="completed", - summary=f"Thread: {compacted_thread}", - ), - message_type="task", - metadata={ - "codex_event_type": "context_compaction", - "codex_room_id": room_id, - "codex_thread_id": compacted_thread, - "codex_turn_id": compacted_turn or None, - }, - ) - except Exception: - logger.debug( - "Failed to emit context compaction event", - exc_info=True, - ) + await send_event_safe( + tools, + content=self._build_task_event_content( + task_id=compacted_turn or None, + task="Codex context compaction", + status="completed", + summary=f"Thread: {compacted_thread}", + ), + message_type="task", + metadata={ + "codex_event_type": "context_compaction", + "codex_room_id": room_id, + "codex_thread_id": compacted_thread, + "codex_turn_id": compacted_turn or None, + }, + log_label="context compaction event", + log_level=logging.DEBUG, + ) continue # --- Phase 4: Aggregated diffs --- @@ -906,26 +1002,21 @@ async def _process_turn_events( and self.config.stream_commentary_events ): # Stream as thought; exclude from final_text. - try: - await tools.send_event( - content=delta, - message_type="thought", - metadata={ - "streaming": True, - "subtype": "commentary", - "codex_item_id": str( - params.get("itemId") or "" - ), - "codex_room_id": room_id, - "codex_thread_id": thread_id, - "codex_turn_id": turn_id, - }, - ) - except Exception: - logger.debug( - "Failed to stream commentary delta", - exc_info=True, - ) + await send_event_safe( + tools, + content=delta, + message_type="thought", + metadata={ + "streaming": True, + "subtype": "commentary", + "codex_item_id": str(params.get("itemId") or ""), + "codex_room_id": room_id, + "codex_thread_id": thread_id, + "codex_turn_id": turn_id, + }, + log_label="commentary delta", + log_level=logging.DEBUG, + ) else: # When streaming is disabled, commentary accumulates # into final_text for backward compatibility. @@ -971,7 +1062,16 @@ async def _process_turn_events( self._token_usage.pop(stale_thread, None) for stale_room in stale_rooms: self._clear_pending_approvals_for_room(stale_room) - break + # Skipped when an earlier "error" notification in this same + # turn already reported one, so one incident isn't posted + # twice -- but the turn still fails either way. + if not failure_reported: + await tools.send_failure( + AgentFailure( + CODEX_PROVIDER, result.turn_error, "transport_closed" + ) + ) + raise TurnResultAlreadyReported(result.turn_error) if event.method == "turn/completed": turn_payload = ( @@ -984,8 +1084,12 @@ async def _process_turn_events( continue result.turn_status = str(turn_payload.get("status") or "failed") result.turn_error = self._extract_turn_error(turn_payload) - # Phase 1: structured error for failed turns - if result.turn_status == "failed" and self.config.structured_errors: + if result.turn_status != "failed": + break + # Skipped when an earlier "error" notification in this same + # turn already reported one, so one incident isn't posted + # twice -- but the turn still fails either way. + if not failure_reported: await self._emit_structured_turn_error( tools=tools, turn_payload=turn_payload, @@ -993,7 +1097,20 @@ async def _process_turn_events( thread_id=thread_id, turn_id=turn_id, ) - break + raise TurnResultAlreadyReported(result.turn_error or "Turn failed") + except TurnResultAlreadyReported as error: + result.turn_status = "failed" + result.turn_error = str(error) + await self._emit_failed_turn_outcome( + tools=tools, + msg=msg, + room_id=room_id, + thread_id=thread_id, + turn_id=turn_id, + result=result, + turn_start=turn_start, + ) + raise except asyncio.TimeoutError: logger.error( "Codex turn timed out after %ss (thread=%s, turn=%s)", @@ -1012,8 +1129,18 @@ async def _process_turn_events( "Failed to send turn/interrupt after timeout", exc_info=True, ) - result.turn_status = "interrupted" - result.turn_error = "Turn timed out" + await tools.send_failure( + AgentFailure( + CODEX_PROVIDER, + f"Codex turn timed out after {self.config.turn_timeout_s}s", + FAILURE_CODE_TIMEOUT, + ) + ) + result.turn_status = "failed" + result.turn_error = ( + f"Codex turn timed out after {self.config.turn_timeout_s}s" + ) + raise TurnResultAlreadyReported("Turn timed out") return result async def on_cleanup(self, room_id: str) -> None: @@ -1144,7 +1271,8 @@ async def _ensure_thread( self._room_threads[room_id] = thread_id self._raw_history_by_room.pop(room_id, None) if Emit.TASK_EVENTS in self.features.emit: - await tools.send_event( + await send_event_safe( + tools, content=self._build_task_event_content( task_id=thread_id, task="Codex thread", @@ -1157,6 +1285,8 @@ async def _ensure_thread( "codex_room_id": room_id, "codex_resumed": True, }, + log_label="thread resumed task event", + log_level=logging.DEBUG, ) return thread_id except CodexJsonRpcError as exc: @@ -1191,7 +1321,8 @@ async def _ensure_thread( self._room_threads[room_id] = thread_id if Emit.TASK_EVENTS in self.features.emit: - await tools.send_event( + await send_event_safe( + tools, content=self._build_task_event_content( task_id=thread_id, task="Codex thread", @@ -1205,6 +1336,8 @@ async def _ensure_thread( "codex_created_at": datetime.now(timezone.utc).isoformat(), "codex_transport": self.config.transport, }, + log_label="thread mapped task event", + log_level=logging.DEBUG, ) return thread_id @@ -1581,29 +1714,24 @@ async def _handle_approval_request( logger.exception("Failed to send approval policy notification") if Emit.THOUGHTS in self.features.emit: - try: - await tools.send_event( - content=( - f"Codex approval request handled automatically ({decision})." - ), - message_type="thought", - metadata={ - "codex_approval_method": event.method, - "codex_approval_type": self._approval_type(event.method), - "codex_approval_options": [ - "accept", - "acceptForSession", - "decline", - ], - }, - ) - except Exception: - # Best-effort telemetry — never fail the turn on thought - # emission failures. - logger.debug( - "Failed to emit approval thought event", - exc_info=True, - ) + # Best-effort telemetry — never fail the turn on thought emission + # failures. + await send_event_safe( + tools, + content=(f"Codex approval request handled automatically ({decision})."), + message_type="thought", + metadata={ + "codex_approval_method": event.method, + "codex_approval_type": self._approval_type(event.method), + "codex_approval_options": [ + "accept", + "acceptForSession", + "decline", + ], + }, + log_label="approval thought event", + log_level=logging.DEBUG, + ) @staticmethod def _turn_usage(usage: CodexTokenUsage | None) -> TurnUsage: @@ -1639,6 +1767,7 @@ async def _emit_turn_outcome( final_text: str, saw_send_message_tool: bool, duration_s: float = 0.0, + include_reply: bool = True, ) -> None: # Look up token usage once for both marker and lifecycle events. usage = self._token_usage.get(thread_id) @@ -1669,7 +1798,8 @@ async def _emit_turn_outcome( metadata["codex_duration_s"] = round(duration_s, 2) if has_usage: metadata.update(usage.to_metadata()) - await tools.send_event( + await send_event_safe( + tools, content=self._build_task_event_content( task_id=turn_id, task="Codex turn", @@ -1678,6 +1808,8 @@ async def _emit_turn_outcome( ), message_type="task", metadata=metadata, + log_label="turn outcome task event", + log_level=logging.DEBUG, ) # Phase 2: Enriched turn lifecycle events @@ -1702,19 +1834,22 @@ async def _emit_turn_outcome( lifecycle_metadata["codex_error"] = turn_error if has_usage: lifecycle_metadata.update(usage.to_metadata()) - try: - await tools.send_event( - content=self._build_task_event_content( - task_id=turn_id, - task="Codex turn lifecycle", - status=turn_status, - summary=f"Duration: {duration_s:.1f}s | Thread: {thread_id}", - ), - message_type="task", - metadata=lifecycle_metadata, - ) - except Exception: - logger.debug("Failed to emit turn lifecycle event", exc_info=True) + await send_event_safe( + tools, + content=self._build_task_event_content( + task_id=turn_id, + task="Codex turn lifecycle", + status=turn_status, + summary=f"Duration: {duration_s:.1f}s | Thread: {thread_id}", + ), + message_type="task", + metadata=lifecycle_metadata, + log_label="turn lifecycle event", + log_level=logging.DEBUG, + ) + + if not include_reply: + return mention = [{"id": msg.sender_id, "name": msg.sender_name or msg.sender_type}] @@ -1724,11 +1859,12 @@ async def _emit_turn_outcome( and final_text.strip() and not saw_send_message_tool ): - await tools.send_message(final_text.strip(), mentions=mention) + await deliver_reply(tools, final_text.strip(), mentions=mention) return if turn_status == "interrupted": - await tools.send_message( + await deliver_reply( + tools, "I stopped before completing this request.", mentions=mention, ) @@ -1739,7 +1875,7 @@ async def _emit_turn_outcome( if not turn_error else f"I couldn't complete this request ({turn_status}): {turn_error}" ) - await tools.send_message(error_text, mentions=mention) + await deliver_reply(tools, error_text, mentions=mention) async def _emit_item_completed_events( self, @@ -2136,23 +2272,44 @@ async def _resolve_manual_approval( net_ctx = params.get("networkContext") or params.get("network_context") if net_ctx: approval_metadata["codex_network_context"] = net_ctx - try: - await tools.send_event( - content=self._build_task_event_content( - task_id=token, - task="Codex approval request", - status="pending", - summary=summary, - ), - message_type="task", - metadata=approval_metadata, - ) - except Exception: - logger.debug( - "Failed to emit approval request task event", - exc_info=True, + await send_event_safe( + tools, + content=self._build_task_event_content( + task_id=token, + task="Codex approval request", + status="pending", + summary=summary, + ), + message_type="task", + metadata=approval_metadata, + log_label="approval request task event", + log_level=logging.DEBUG, + ) + try: + await tools.send_message(approval_msg, mentions=mention) + except Exception: + # The room was never notified, so waiting out the full + # approval_wait_timeout_s would misreport a Band delivery + # hiccup as a genuine human-decision timeout. Report it now, + # rather than letting it silently decline with no signal at + # all, same as every other failure path in this file. Re-raise + # (rather than returning "decline" here) so the caller's own + # except-block attributes this to "system_fallback" instead + # of crediting/blaming the human sender for a decision they + # were never actually notified about. + logger.exception( + "Failed to notify room %s about pending approval %s", + room_id, + token, + ) + await tools.send_failure( + AgentFailure( + CODEX_PROVIDER, + "Failed to notify the room about a pending approval " + "request; defaulting to decline.", ) - await tools.send_message(approval_msg, mentions=mention) + ) + raise decision_raw = await asyncio.wait_for( pending.future, timeout=self.config.approval_wait_timeout_s, @@ -2240,8 +2397,13 @@ async def _handle_error_event( room_id: str, thread_id: str, turn_id: str | None, - ) -> None: - """Handle an ``error`` notification from Codex.""" + ) -> bool: + """Handle an ``error`` notification from Codex. + + Returns whether a failure was actually reported, so the turn loop can + skip a redundant second report if ``turn/completed`` also arrives with + a failed status for the same incident. + """ error_obj = params.get("error") or {} if isinstance(error_obj, dict): error_msg = error_obj.get("message", "") @@ -2263,29 +2425,15 @@ async def _handle_error_event( turn_id, error_msg, ) - return + return False logger.error("Codex error: %s", error_msg) - if self.config.structured_errors: - content, err_meta = build_structured_error_metadata( - error_obj, thread_id=thread_id, turn_id=turn_id - ) - err_meta["codex_room_id"] = room_id - await tools.send_event( - content=content or f"Codex error: {error_msg}", - message_type="error", - metadata=err_meta, - ) - else: - await tools.send_event( - content=f"Codex error: {error_msg}", - message_type="error", - metadata={ - "codex_room_id": room_id, - "codex_thread_id": thread_id, - "codex_turn_id": turn_id, - }, + await tools.send_failure( + build_agent_failure( + error_obj, thread_id=thread_id, turn_id=turn_id, room_id=room_id ) + ) + return True async def _emit_structured_turn_error( self, @@ -2299,19 +2447,21 @@ async def _emit_structured_turn_error( """Emit a structured error event when turn/completed reports failure.""" error = turn_payload.get("error") if not isinstance(error, dict): - return - content, err_meta = build_structured_error_metadata( - error, thread_id=thread_id, turn_id=turn_id + # A falsy scalar (``""``, ``0``, ``None``) has no useful message to + # carry; build_agent_failure's own fallback covers it uniformly + # instead of shipping a degenerate literal string like "None". + error = {"message": str(error)} if error else {} + logger.error( + "Codex turn failed (thread=%s, turn=%s): %s", + thread_id, + turn_id, + error.get("message", ""), ) - err_meta["codex_room_id"] = room_id - try: - await tools.send_event( - content=content, - message_type="error", - metadata=err_meta, + await tools.send_failure( + build_agent_failure( + error, thread_id=thread_id, turn_id=turn_id, room_id=room_id ) - except Exception: - logger.debug("Failed to emit structured turn error", exc_info=True) + ) # ------------------------------------------------------------------ # Phase 2: Plan step tracking @@ -2331,24 +2481,24 @@ async def _forward_plan_steps( if not steps: return step_dicts = [{"step": s.step, "status": s.status} for s in steps] - try: - await tools.send_event( - content=self._build_task_event_content( - task_id=turn_id, - task="Codex plan", - status="updated", - summary=f"{len(steps)} steps", - ), - message_type="task", - metadata={ - "codex_plan_steps": step_dicts, - "codex_room_id": room_id, - "codex_thread_id": thread_id, - "codex_turn_id": turn_id, - }, - ) - except Exception: - logger.debug("Failed to forward plan steps", exc_info=True) + await send_event_safe( + tools, + content=self._build_task_event_content( + task_id=turn_id, + task="Codex plan", + status="updated", + summary=f"{len(steps)} steps", + ), + message_type="task", + metadata={ + "codex_plan_steps": step_dicts, + "codex_room_id": room_id, + "codex_thread_id": thread_id, + "codex_turn_id": turn_id, + }, + log_label="plan steps", + log_level=logging.DEBUG, + ) # ------------------------------------------------------------------ # Phase 4: Token usage & diffs @@ -2379,14 +2529,14 @@ async def _emit_token_usage_event( metadata = usage.to_metadata() metadata["codex_thread_id"] = thread_id metadata["codex_room_id"] = room_id - try: - await tools.send_event( - content=usage.format_summary(), - message_type="task", - metadata=metadata, - ) - except Exception: - logger.debug("Failed to emit token usage event", exc_info=True) + await send_event_safe( + tools, + content=usage.format_summary(), + message_type="task", + metadata=metadata, + log_label="token usage event", + log_level=logging.DEBUG, + ) async def _forward_diff_event( self, @@ -2442,19 +2592,19 @@ async def _forward_diff_event( metadata["codex_diff_truncated"] = True metadata["codex_diff_original_length"] = original_length metadata["codex_diff_original_bytes"] = original_byte_length - try: - await tools.send_event( - content=self._build_task_event_content( - task_id=turn_id, - task="Codex diff", - status="updated", - summary=summary, - ), - message_type="task", - metadata=metadata, - ) - except Exception: - logger.debug("Failed to forward diff event", exc_info=True) + await send_event_safe( + tools, + content=self._build_task_event_content( + task_id=turn_id, + task="Codex diff", + status="updated", + summary=summary, + ), + message_type="task", + metadata=metadata, + log_label="diff event", + log_level=logging.DEBUG, + ) # ------------------------------------------------------------------ # Phase 1: Approval audit trail @@ -2515,27 +2665,27 @@ async def _emit_approval_audit_event( """Emit a task event for an approval decision.""" if Emit.TASK_EVENTS not in self.features.emit: return - try: - await tools.send_event( - content=self._build_task_event_content( - task_id=str(entry.request_id), - task="Codex approval", - status=entry.decision, - summary=entry.summary, - ), - message_type="task", - metadata={ - "codex_event_type": "approval_resolution", - "codex_approval_method": entry.method, - "codex_approval_decision": entry.decision, - "codex_decided_by": entry.decided_by, - "codex_session_level": entry.session_level, - "codex_room_id": room_id, - "codex_timestamp": entry.timestamp, - }, - ) - except Exception: - logger.debug("Failed to emit approval audit event", exc_info=True) + await send_event_safe( + tools, + content=self._build_task_event_content( + task_id=str(entry.request_id), + task="Codex approval", + status=entry.decision, + summary=entry.summary, + ), + message_type="task", + metadata={ + "codex_event_type": "approval_resolution", + "codex_approval_method": entry.method, + "codex_approval_decision": entry.decision, + "codex_decided_by": entry.decided_by, + "codex_session_level": entry.session_level, + "codex_room_id": room_id, + "codex_timestamp": entry.timestamp, + }, + log_label="approval audit event", + log_level=logging.DEBUG, + ) async def _handle_local_command( self, @@ -2550,7 +2700,8 @@ async def _handle_local_command( mention = [{"id": msg.sender_id, "name": msg.sender_name or msg.sender_type}] if command == "help": - await tools.send_message( + await deliver_reply( + tools, "Codex commands: " "`/status`, `/model`, `/models`, `/model list`, `/models list`, `/model `, " "`/reasoning [none|minimal|low|medium|high|xhigh]`, " @@ -2587,13 +2738,14 @@ async def _handle_local_command( f"- token_usage: {usage_line}\n" f"- turn_task_markers: {self.config.emit_turn_task_markers}" ) - await tools.send_message(status_text, mentions=mention) + await deliver_reply(tools, status_text, mentions=mention) return True if command in {"model", "models"}: model_arg = args.strip() if not model_arg: - await tools.send_message( + await deliver_reply( + tools, "Current model: " f"`{self._selected_model or 'unknown'}` " f"(configured: `{self.config.model or 'auto'}`). " @@ -2611,12 +2763,14 @@ async def _handle_local_command( preview = ", ".join(models[:10]) if len(models) > 10: preview += ", ..." - await tools.send_message( + await deliver_reply( + tools, f"Available models ({len(models)}): {preview}", mentions=mention, ) else: - await tools.send_message( + await deliver_reply( + tools, "No visible models returned by Codex app-server.", mentions=mention, ) @@ -2624,7 +2778,8 @@ async def _handle_local_command( self.config.model = model_arg self._selected_model = model_arg - await tools.send_message( + await deliver_reply( + tools, f"Model override set to `{model_arg}` for subsequent turns.", mentions=mention, ) @@ -2633,7 +2788,8 @@ async def _handle_local_command( if command == "reasoning": effort_arg = args.strip().lower() if not effort_arg: - await tools.send_message( + await deliver_reply( + tools, f"Current reasoning effort: `{self.config.reasoning_effort or 'default'}`. " f"Summary: `{self.config.reasoning_summary or 'default'}`. " f"Use `/reasoning <{'|'.join(sorted(_REASONING_EFFORTS))}>` to override.", @@ -2641,14 +2797,16 @@ async def _handle_local_command( ) return True if effort_arg not in _REASONING_EFFORTS: - await tools.send_message( + await deliver_reply( + tools, f"Invalid reasoning effort `{effort_arg}`. " f"Valid values: {', '.join(sorted(_REASONING_EFFORTS))}.", mentions=mention, ) return True self.config.reasoning_effort = effort_arg # type: ignore[assignment] # Literal narrowed by Pydantic validation - await tools.send_message( + await deliver_reply( + tools, f"Reasoning effort set to `{effort_arg}` for subsequent turns.", mentions=mention, ) @@ -2657,7 +2815,8 @@ async def _handle_local_command( # --- Phase 1: /sandbox and /permissions commands --- if command == "sandbox": if self.config.sandbox_policy is not None: - await tools.send_message( + await deliver_reply( + tools, "Cannot override sandbox: a `sandbox_policy` is configured. " "Remove `sandbox_policy` from config to use per-room `/sandbox` overrides.", mentions=mention, @@ -2666,7 +2825,8 @@ async def _handle_local_command( mode_arg = args.strip() if not mode_arg: effective = self._effective_sandbox(room_id) or "default" - await tools.send_message( + await deliver_reply( + tools, f"Current sandbox: `{effective}`. " "Use `/sandbox ` to change.", mentions=mention, @@ -2678,7 +2838,8 @@ async def _handle_local_command( confirm_flag = "--confirm" in tokens mode_tokens = [tok for tok in tokens if tok != "--confirm"] if len(mode_tokens) != 1: - await tools.send_message( + await deliver_reply( + tools, "Usage: `/sandbox " "[--confirm]`.", mentions=mention, @@ -2687,14 +2848,16 @@ async def _handle_local_command( mode_token = mode_tokens[0] normalized = self._normalize_sandbox_mode(mode_token) if normalized is None: - await tools.send_message( + await deliver_reply( + tools, f"Invalid sandbox mode `{mode_token}`. " "Valid: read-only, workspace-write, danger-full-access.", mentions=mention, ) return True if normalized == "danger-full-access" and not confirm_flag: - await tools.send_message( + await deliver_reply( + tools, "Escalating to `danger-full-access` removes all sandbox " "restrictions. Re-run with `--confirm` to proceed:\n" "`/sandbox danger-full-access --confirm`", @@ -2709,7 +2872,8 @@ async def _handle_local_command( msg.sender_name or msg.sender_type or "unknown", ) self._sandbox_overrides[room_id] = normalized - await tools.send_message( + await deliver_reply( + tools, f"Sandbox mode set to `{normalized}` for subsequent turns in this room.", mentions=mention, ) @@ -2733,7 +2897,7 @@ async def _handle_local_command( f" - [{entry.timestamp}] {entry.method}: " f"{entry.decision} by {entry.decided_by}" ) - await tools.send_message("\n".join(lines), mentions=mention) + await deliver_reply(tools, "\n".join(lines), mentions=mention) return True # --- Phase 2: /threads, /thread info, /thread archive --- @@ -2742,22 +2906,22 @@ async def _handle_local_command( if command == "threads" or not subcommand: # List all room->thread mappings if not self._room_threads: - await tools.send_message( - "No active thread mappings.", mentions=mention + await deliver_reply( + tools, "No active thread mappings.", mentions=mention ) return True lines = ["Active thread mappings:"] for rid, tid in self._room_threads.items(): current = " (current)" if rid == room_id else "" lines.append(f"- room `{rid}` → thread `{tid}`{current}") - await tools.send_message("\n".join(lines), mentions=mention) + await deliver_reply(tools, "\n".join(lines), mentions=mention) return True if subcommand == "info": mapped_thread = self._room_threads.get(room_id) if not mapped_thread: - await tools.send_message( - "No thread mapped for this room.", mentions=mention + await deliver_reply( + tools, "No thread mapped for this room.", mentions=mention ) return True usage = self._token_usage.get(mapped_thread) @@ -2772,7 +2936,7 @@ async def _handle_local_command( f"- room_id: {room_id}\n" f"- token_usage: {usage_line}" ) - await tools.send_message(info_text, mentions=mention) + await deliver_reply(tools, info_text, mentions=mention) return True if subcommand == "archive": @@ -2781,7 +2945,8 @@ async def _handle_local_command( self._token_usage.pop(mapped_thread or "", None) self._raw_history_by_room.pop(room_id, None) self._needs_history_injection.discard(room_id) - await tools.send_message( + await deliver_reply( + tools, f"Thread `{mapped_thread or 'none'}` archived. " "A new thread will be created on next message.", mentions=mention, @@ -2794,19 +2959,22 @@ async def _handle_local_command( if command == "usage": mapped_thread = self._room_threads.get(room_id) if not mapped_thread: - await tools.send_message( + await deliver_reply( + tools, "No thread mapped for this room — no usage data.", mentions=mention, ) return True usage = self._token_usage.get(mapped_thread) if not usage or usage.total_tokens == 0: - await tools.send_message( + await deliver_reply( + tools, "No token usage recorded for this thread.", mentions=mention, ) return True - await tools.send_message( + await deliver_reply( + tools, f"Thread `{mapped_thread}` — {usage.format_summary()}", mentions=mention, ) @@ -2828,21 +2996,22 @@ async def _handle_approval_command( if command == "approvals": if not pending: - await tools.send_message("No pending approvals.", mentions=mention) + await deliver_reply(tools, "No pending approvals.", mentions=mention) return True lines = ["Pending approvals:"] now = datetime.now(timezone.utc) for token, item in list(pending.items()): age_s = int((now - item.created_at).total_seconds()) lines.append(f"- {token}: {item.summary} ({age_s}s)") - await tools.send_message("\n".join(lines), mentions=mention) + await deliver_reply(tools, "\n".join(lines), mentions=mention) return True if command not in {"approve", "decline", "approve-session"}: return False if not pending: - await tools.send_message( + await deliver_reply( + tools, "No pending approvals to resolve.", mentions=mention, ) @@ -2853,7 +3022,8 @@ async def _handle_approval_command( selected = pending.get(token) if selected is None: available = ", ".join(sorted(pending.keys())) - await tools.send_message( + await deliver_reply( + tools, f"Unknown approval id `{token}`. Pending: {available}", mentions=mention, ) @@ -2862,7 +3032,8 @@ async def _handle_approval_command( token, selected = next(iter(pending.items())) else: available = ", ".join(sorted(pending.keys())) - await tools.send_message( + await deliver_reply( + tools, "Multiple approvals pending. " f"Use `/{command} `. Pending: {available}", mentions=mention, @@ -2878,7 +3049,8 @@ async def _handle_approval_command( # an empty string in _session_approved or report a misleading # "Future `` requests will be auto-approved" message to the user. if is_session and not selected.session_key: - await tools.send_message( + await deliver_reply( + tools, f"Approval `{token}` cannot be resolved as session-level: " "this request has no command signature to match against. " f"Use `/approve {token}` for a one-shot approval instead.", @@ -2899,13 +3071,15 @@ async def _handle_approval_command( # Session-level: register the session key for auto-approval if is_session: self._record_session_approval(room_id, selected.session_key) - await tools.send_message( + await deliver_reply( + tools, f"Approval `{token}` resolved as `acceptForSession` (session-level). " f"Future `{selected.session_key}` requests will be auto-approved.", mentions=mention, ) else: - await tools.send_message( + await deliver_reply( + tools, f"Approval `{token}` resolved as `{decision_value}`.", mentions=mention, ) diff --git a/src/band/adapters/copilot_sdk.py b/src/band/adapters/copilot_sdk.py index 833415985..a1a60db67 100644 --- a/src/band/adapters/copilot_sdk.py +++ b/src/band/adapters/copilot_sdk.py @@ -14,6 +14,7 @@ from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, ClassVar, Literal +from band_sdk_core import AgentFailure from pydantic import ValidationError from band.converters.copilot_sdk import ( @@ -21,7 +22,13 @@ CopilotSDKHistoryConverter, CopilotSDKSessionState, ) +from band.core.delivery import ( + DeliveryFailedError, + deliver_reply, + reraise_delivery_cause, +) from band.core.exceptions import BandConfigError +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE, send_event_safe from band.core.simple_adapter import SimpleAdapter from band.core.tool_filter import filter_tool_schemas from band.core.types import Capability, Emit, MessageType, ToolEventKey, TurnUsage @@ -88,6 +95,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "copilot_sdk" + @dataclass(frozen=True) class CopilotSDKAdapterConfig: @@ -403,9 +412,16 @@ async def on_message( # Same-session calls must not interleave; other rooms run concurrently. async with self._session_manager.turn_lock(room_id): - session, inject_text = await self._obtain_session( - room_id, history, tools, is_session_bootstrap=is_session_bootstrap - ) + try: + session, inject_text = await self._obtain_session( + room_id, history, tools, is_session_bootstrap=is_session_bootstrap + ) + except Exception: + logger.exception("Room %s: Copilot session setup failed", room_id) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) + raise prompt = self._compose_prompt( msg, participants_msg, @@ -425,12 +441,14 @@ async def on_message( self._turn_state[room_id] = turn try: final_text = await self._run_turn(session, prompt, turn) - except Exception as exc: + except Exception: logger.exception("Room %s: Copilot turn failed", room_id) # Abort any work the runtime is still doing for this turn and # drop the session; the next message resumes it fresh by id. await self._session_manager.evict_session(room_id) - await self._report_error(tools, str(exc)) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) raise finally: self._turn_state.pop(room_id, None) @@ -444,13 +462,19 @@ async def on_message( # Session errors raise out of send_and_wait, so a None here # with no room output means the model genuinely said nothing. if final_text is None and not turn.replied_in_room: - await self._report_error(tools, "no assistant reply") + logger.warning("Room %s: Copilot turn produced no reply", room_id) + await tools.send_failure(AgentFailure(_PROVIDER, "no assistant reply")) raise RuntimeError("Copilot turn produced no reply") # The turn may already have replied into the room; sending its # final text too would duplicate the reply. if final_text and not turn.replied_in_room: - await tools.send_message(final_text, mentions=[turn.sender_mention]) + try: + await deliver_reply( + tools, final_text, mentions=[turn.sender_mention] + ) + except DeliveryFailedError as e: + reraise_delivery_cause(e) await self._persist_session_id(room_id, tools) @@ -808,7 +832,7 @@ async def _report_tool_call( invocation: ToolInvocation, arguments: dict[str, Any], ) -> None: - await self._send_event_safe( + await send_event_safe( room_tools, json.dumps( { @@ -828,7 +852,7 @@ async def _report_tool_result( invocation: ToolInvocation, output: str, ) -> None: - await self._send_event_safe( + await send_event_safe( room_tools, json.dumps( { @@ -920,7 +944,7 @@ def _usage_from_event(data: AssistantUsageData) -> TurnUsage: async def _emit_thoughts(self, turn: TurnState, tools: AgentToolsProtocol) -> None: if Emit.THOUGHTS in self.features.emit: for reasoning in turn.reasonings.values(): - await self._send_event_safe(tools, reasoning, MessageType.THOUGHT) + await send_event_safe(tools, reasoning, MessageType.THOUGHT) async def _persist_session_id( self, room_id: str, tools: AgentToolsProtocol @@ -932,7 +956,7 @@ async def _persist_session_id( """ ids = self._ids(room_id) if ids.current and ids.persisted != ids.current: - sent = await self._send_event_safe( + sent = await send_event_safe( tools, "Copilot SDK session", MessageType.TASK, @@ -942,26 +966,3 @@ async def _persist_session_id( # Only mark persisted on success so a transient send failure # is retried next turn instead of silently losing resume. ids.persisted = ids.current - - async def _send_event_safe( - self, - tools: AgentToolsProtocol, - content: str, - message_type: MessageType, - metadata: dict[str, Any] | None = None, - ) -> bool: - """Send a platform event, downgrading failures to a warning. - - Returns True when the event was accepted by the platform. - """ - try: - await tools.send_event( - content=content, message_type=message_type, metadata=metadata - ) - except Exception as exc: - logger.warning("Failed to send %s event: %s", message_type, exc) - return False - return True - - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - await self._send_event_safe(tools, f"Error: {error}", MessageType.ERROR) diff --git a/src/band/adapters/crewai.py b/src/band/adapters/crewai.py index ba7047123..8e0f63468 100644 --- a/src/band/adapters/crewai.py +++ b/src/band/adapters/crewai.py @@ -14,9 +14,14 @@ from contextvars import ContextVar from typing import ClassVar, TYPE_CHECKING, Any +from band_sdk_core import AgentFailure from typing_extensions import Unpack -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, + TurnResultAlreadyReported, +) from band.core.simple_adapter import SimpleAdapter from band.core.types import Capability, Emit, FeatureKwargs, PlatformMessage from band.converters.crewai import CrewAIHistoryConverter, CrewAIMessages @@ -36,6 +41,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "crewai" + # Context variable for thread-safe room context access. # Set automatically when processing messages, accessed by tools. @@ -295,9 +302,9 @@ async def on_message( logger.debug("Handling message %s in room %s", msg.id, room_id) if not self._crewai_agent: - raise RuntimeError( - "CrewAI agent not initialized - ensure on_started() was called" - ) + message = "CrewAI agent not initialized - ensure on_started() was called" + await tools.send_failure(AgentFailure(_PROVIDER, message)) + raise RuntimeError(message) # Set context variable for tool access (thread-safe room context). # Wrap in try/finally immediately to ensure cleanup even if code @@ -410,13 +417,16 @@ async def _process_message( ) except Exception as e: - # An empty response is benign only once some tool ran this turn -- - # otherwise the model's very first call came back empty, which is + # An empty response is benign only once some tool ran this turn. + # Reaching here with no tool activity means the first-call retry + # also came back empty (or this was a different failure), which is # indistinguishable from a genuine provider failure and must keep # failing the delivery so the platform retries it. if not (_is_empty_llm_response(e) and reply_tracker.any_tool_ran): logger.error("Error processing message: %s", e, exc_info=True) - await self._report_error(tools, str(e)) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) raise # Keep the exception text: it is the only record that CrewAI raised, # and this turn is no longer marked failed for the runtime to log. @@ -433,21 +443,23 @@ async def _process_message( ) if not reply_tracker.did_productive_work: - # Warn, not debug: nothing reached the room, and the delivery is - # still acked as processed, so this log is the only operator signal. logger.warning( "Room %s: CrewAI turn produced nothing for the room", room_id ) - await self._report_error( - tools, - missing_reply_error( - "CrewAI", - detail=( - "Repeated tool failures may also have exhausted " - f"max_iter={self.max_iter}." - ), + detail = missing_reply_error( + "CrewAI", + detail=( + "Repeated tool failures may also have exhausted " + f"max_iter={self.max_iter}." ), ) + await tools.send_failure(AgentFailure(_PROVIDER, detail)) + if not reply_tracker.any_tool_ran: + # Some tool activity (even read-only) means the turn did what it + # was asked and correctly had nothing left to say -- report but + # don't fail the delivery. Only true silence, no tool call at + # all, is a genuine no-response failure worth a retry. + raise TurnResultAlreadyReported(detail) logger.info( "Room %s: CrewAI turn over for %s (output=%s chars, history=%s)", @@ -504,10 +516,3 @@ async def _kickoff_with_empty_response_retry( retry_exc, ) raise - - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - """Send error event (best effort).""" - try: - await tools.send_event(content=f"Error: {error}", message_type="error") - except Exception as e: - logger.warning("Failed to send error event: %s", e) diff --git a/src/band/adapters/crewai_flow.py b/src/band/adapters/crewai_flow.py index ec574e9c2..a020d415a 100644 --- a/src/band/adapters/crewai_flow.py +++ b/src/band/adapters/crewai_flow.py @@ -26,6 +26,7 @@ from typing import Any, Callable, Literal, Protocol, Union, runtime_checkable from uuid import UUID +from band_sdk_core import AgentFailure from pydantic import BaseModel, ConfigDict, Field, ValidationError, model_validator from typing_extensions import Unpack @@ -937,15 +938,17 @@ async def record_waiting(self, reason: str) -> None: ) async def record_failed(self, error: CrewAIFlowError) -> None: - # Best-effort error event for visibility, then the task event. - try: - await self._tools.send_event( - content=f"flow error: {error.code}: {error.message}"[:500], - message_type="error", - metadata={"error": error.model_dump()}, - ) - except Exception: # noqa: BLE001 - logger.warning("Failed to emit error event", exc_info=True) + # Best-effort failure event for visibility, then the task event. The + # room-visible message is capped like every other room post in this + # file (e.g. record_waiting) -- error.message can embed an unbounded + # value (e.g. a full participant-id list from an ambiguous-identity + # error) -- so the untruncated text is preserved in detail for + # structured consumers reading the "error"-typed event. + message = error.message[:500] + detail = error.message if message != error.message else None + await self._tools.send_failure( + AgentFailure("crewai_flow", message, error.code, detail) + ) await self._send_event( content=f"failed:{error.code}", message_type="task", diff --git a/src/band/adapters/gemini.py b/src/band/adapters/gemini.py index 842c063e2..f9e6e20b2 100644 --- a/src/band/adapters/gemini.py +++ b/src/band/adapters/gemini.py @@ -9,6 +9,7 @@ from typing import Any, ClassVar, cast import httpx +from band_sdk_core import AgentFailure from pydantic import ValidationError from typing_extensions import Unpack @@ -24,7 +25,7 @@ ) from e from band.core.exceptions import BandConfigError -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE, AgentToolsProtocol from band.core.simple_adapter import SimpleAdapter from band.core.tool_filter import sanitize_tool_schema from band.core.types import ( @@ -53,6 +54,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "gemini" + def _image_function_response_parts( result: dict[str, Any], @@ -71,6 +74,18 @@ def _image_function_response_parts( return parts +def _to_agent_failure(e: Exception) -> AgentFailure: + """Parse a turn-ending exception into the shared provider-failure shape. + + ``ServerError`` carries an HTTP status and message that a plain + exception's text alone does not. + """ + if isinstance(e, ServerError): + status = e.status if e.status is None else str(e.status) + return AgentFailure(_PROVIDER, str(e), status, e.message) + return AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + + class GeminiAdapter(SimpleAdapter[GeminiMessages]): """ Gemini SDK adapter using SimpleAdapter pattern. @@ -240,10 +255,12 @@ async def on_message( try: while True: if tool_rounds >= self.max_tool_rounds: - raise RuntimeError( + message = ( f"Exceeded max tool rounds ({self.max_tool_rounds}) " f"in room {room_id}" ) + await tools.send_failure(AgentFailure(_PROVIDER, message)) + raise RuntimeError(message) try: response = await self._call_gemini( @@ -251,7 +268,7 @@ async def on_message( ) except Exception as e: logger.exception("Error calling Gemini: %s", e) - await self._report_error(tools, str(e)) + await tools.send_failure(_to_agent_failure(e)) raise turn_usage = turn_usage + self._usage_from_response(response) @@ -591,10 +608,3 @@ async def _process_function_calls( ) return tool_response_parts - - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - """Send error event (best effort).""" - try: - await tools.send_event(content=f"Error: {error}", message_type="error") - except Exception as e: - logger.warning("Failed to send error event: %s", e) diff --git a/src/band/adapters/google_adk.py b/src/band/adapters/google_adk.py index eac5aa15a..7cbd79504 100644 --- a/src/band/adapters/google_adk.py +++ b/src/band/adapters/google_adk.py @@ -15,10 +15,11 @@ import uuid from typing import ClassVar, TYPE_CHECKING, Any, cast +from band_sdk_core import AgentFailure from pydantic import ValidationError from typing_extensions import Unpack -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE, AgentToolsProtocol from band.core.simple_adapter import SimpleAdapter from band.core.tool_filter import sanitize_tool_schema from band.core.types import ( @@ -474,14 +475,18 @@ async def on_message( # Safety: ensure history exists even if not first message self._room_history[room_id] = [] - # A fresh runner is created per message because InMemoryRunner - # accumulates session history internally and tool schemas may change - # between calls. History is injected as a text transcript instead. - runner = self._create_runner(tools) # Per-turn usage, summed across the event stream below. Initialized # outside the try so the finally can emit whatever accumulated. turn_usage = TurnUsage() + # None until the try's construction succeeds, so the finally's close() + # has nothing to do if runner construction itself is what failed. + runner: InMemoryRunner | None = None try: + # A fresh runner is created per message because InMemoryRunner + # accumulates session history internally and tool schemas may change + # between calls. History is injected as a text transcript instead. + runner = self._create_runner(tools) + # Always create a new session ID — each runner is fresh, so there # is no in-memory state to resume. The ID is stored for cleanup # tracking. The session must be pre-created in the runner's @@ -576,9 +581,11 @@ async def on_message( "Room %s: ADK agent completed with final response", room_id, ) - except Exception as e: + except Exception: logger.exception("Error running ADK agent in room %s", room_id) - await self._report_error(tools, str(e)) + await tools.send_failure( + AgentFailure("google_adk", GENERIC_PROVIDER_FAILURE_MESSAGE) + ) raise finally: # Emit before close so a close() failure can't drop the usage, but @@ -587,7 +594,8 @@ async def on_message( # No-op unless Emit.USAGE is on; best-effort, never raises. await self.emit_usage(tools, turn_usage) finally: - await runner.close() + if runner is not None: + await runner.close() # Accumulate message history for future transcript injection self._room_history[room_id].append( @@ -724,10 +732,3 @@ async def _report_event(self, event: Any, tools: AgentToolsProtocol) -> None: ) except Exception as e: logger.warning("Failed to send tool_result event: %s", e) - - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - """Send error event (best effort).""" - try: - await tools.send_event(content=f"Error: {error}", message_type="error") - except Exception as e: - logger.warning("Failed to send error event: %s", e) diff --git a/src/band/adapters/langgraph.py b/src/band/adapters/langgraph.py index 4e3b0e8bc..027077de3 100644 --- a/src/band/adapters/langgraph.py +++ b/src/band/adapters/langgraph.py @@ -8,11 +8,12 @@ from collections import OrderedDict from typing import ClassVar, TYPE_CHECKING, Any, Callable +from band_sdk_core import AgentFailure from langgraph.checkpoint.memory import InMemorySaver from langgraph.pregel import Pregel from typing_extensions import Unpack -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE, AgentToolsProtocol from band.core.simple_adapter import SimpleAdapter from band.core.types import ( Capability, @@ -284,62 +285,6 @@ async def on_message( """Handle message with LangGraph.""" logger.info("[HANDLE] Message %s in room %s", msg.id, room_id) - # Get LangChain tools - lc_tools = ( - langchain_tools.agent_tools_to_langchain( - tools, - features=self.features, - ) - + self.additional_tools - ) - - # Build or get graph - if self.graph_factory: - graph = self.graph_factory(lc_tools) - else: - graph = self._static_graph - - if not graph: - raise RuntimeError("No graph available") - - checkpointer = getattr(graph, "checkpointer", None) or self._simple_checkpointer - if checkpointer is not None: - self._room_checkpointers[room_id] = checkpointer - - # Build messages - messages: list[Any] = [] - - # Session bootstrap: prepend the rendered system prompt and hydrate - # platform history exactly once per room. After that, the LangGraph - # checkpointer carries the system message and prior turns forward and - # we just append the new user turn. - should_mark_bootstrapped = False - if is_session_bootstrap and room_id not in self._bootstrapped_rooms: - checkpointer_already_has_messages = ( - checkpointer is not None - and await self._checkpointer_has_messages(checkpointer, room_id) - ) - if not checkpointer_already_has_messages: - if self._inject_system_prompt and self._system_prompt: - messages.append(("system", self._system_prompt)) - if history: - messages.extend(history) # Already converted by history_converter - should_mark_bootstrapped = True - - # Inject metadata updates as user messages with [System]: prefix. - # Many LLM providers (including Anthropic) require a single system - # message at the start; additional system messages scattered through - # the conversation cause errors and kill provider cache savings. - if participants_msg: - messages.append(("user", f"[System]: {participants_msg}")) - - if contacts_msg: - messages.append(("user", f"[System]: {contacts_msg}")) - - messages.append(("user", msg.format_for_llm())) - - graph_input = {"messages": messages} - # Usage is reported per model call on the stream; a turn may make several # (a tool loop), so sum across every on_chat_model_end into one TurnUsage, # emitted on every exit via the finally. Gated outside the loop: the @@ -348,6 +293,66 @@ async def on_message( track_usage = Emit.USAGE in self.features.emit turn_usage = TurnUsage() try: + # Get LangChain tools + lc_tools = ( + langchain_tools.agent_tools_to_langchain( + tools, + features=self.features, + ) + + self.additional_tools + ) + + # Build or get graph + if self.graph_factory: + graph = self.graph_factory(lc_tools) + else: + graph = self._static_graph + + if not graph: + raise RuntimeError("No graph available") + + checkpointer = ( + getattr(graph, "checkpointer", None) or self._simple_checkpointer + ) + if checkpointer is not None: + self._room_checkpointers[room_id] = checkpointer + + # Build messages + messages: list[Any] = [] + + # Session bootstrap: prepend the rendered system prompt and hydrate + # platform history exactly once per room. After that, the LangGraph + # checkpointer carries the system message and prior turns forward and + # we just append the new user turn. + should_mark_bootstrapped = False + if is_session_bootstrap and room_id not in self._bootstrapped_rooms: + checkpointer_already_has_messages = ( + checkpointer is not None + and await self._checkpointer_has_messages(checkpointer, room_id) + ) + if not checkpointer_already_has_messages: + if self._inject_system_prompt and self._system_prompt: + messages.append(("system", self._system_prompt)) + if history: + messages.extend( + history + ) # Already converted by history_converter + should_mark_bootstrapped = True + + # Inject metadata updates as user messages with [System]: prefix. + # Many LLM providers (including Anthropic) require a single system + # message at the start; additional system messages scattered through + # the conversation cause errors and kill provider cache savings. + if participants_msg: + messages.append(("user", f"[System]: {participants_msg}")) + + if contacts_msg: + messages.append(("user", f"[System]: {contacts_msg}")) + + messages.append(("user", msg.format_for_llm())) + + graph_input = {"messages": messages} + async for event in graph.astream_events( graph_input, config={ @@ -376,17 +381,14 @@ async def on_message( except Exception: logger.exception("Error processing message %s", msg.id) - try: - # Keep the user-facing payload generic; the full traceback is - # in the agent log via logger.exception above. Tool/error - # internals can include DB strings, paths, and tokens that - # should not surface in chat. - await tools.send_event( - content="Internal error while processing message; see agent logs.", - message_type="error", - ) - except Exception: - logger.exception("Failed to report error event for message %s", msg.id) + # Keep the user-facing payload generic; the full traceback is in + # the agent log via logger.exception above. Tool/error internals + # can include DB strings, paths, and tokens that should not + # surface in chat -- code/detail stay unset, never populated from + # the caught exception. + await tools.send_failure( + AgentFailure("langgraph", GENERIC_PROVIDER_FAILURE_MESSAGE) + ) raise finally: # No-op unless Emit.USAGE is on; best-effort, never raises. diff --git a/src/band/adapters/letta.py b/src/band/adapters/letta.py index 91c824a32..b045d1124 100644 --- a/src/band/adapters/letta.py +++ b/src/band/adapters/letta.py @@ -9,10 +9,21 @@ from datetime import datetime, timezone from typing import ClassVar, Any +from band_sdk_core import AgentFailure from typing_extensions import Unpack from band.converters.letta import LettaHistoryConverter, LettaSessionState -from band.core.protocols import AgentToolsProtocol +from band.core.delivery import ( + DeliveryFailedError, + deliver_reply, + reraise_delivery_cause, +) +from band.core.protocols import ( + FAILURE_CODE_TIMEOUT, + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, + TurnResultAlreadyReported, +) from band.core.simple_adapter import SimpleAdapter from band.core.types import ( AdapterFeatures, @@ -48,6 +59,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "letta" + @dataclass class RoomContext: @@ -267,8 +280,9 @@ async def on_message( """Handle incoming message via Letta API with MCP tools.""" if not self._client: logger.error("Letta client not initialized, dropping message %s", msg.id) - await self._report_error(tools, "Letta adapter not initialized") - return + message = "Letta adapter not initialized" + await tools.send_failure(AgentFailure(_PROVIDER, message)) + raise RuntimeError(message) # Lock only protects MCP/agent setup, not the full message path. # This allows concurrent rooms to process messages in parallel. Both @@ -284,8 +298,10 @@ async def on_message( await self._ensure_agent(room_id, history, tools) except Exception as e: logger.exception("Room %s: Failed to prepare Letta session: %s", room_id, e) - await self._report_error(tools, str(e)) - return + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) + raise await self._handle_message( msg=msg, @@ -311,8 +327,9 @@ async def _handle_message( """Run one Letta turn: resolve the room, compose the message, send.""" if (room_ctx := await self._room_context(room_id, history, tools)) is None: logger.error("Room %s: No Letta agent context, dropping message", room_id) - await self._report_error(tools, "Letta agent context unavailable") - return + message = "Letta agent context unavailable" + await tools.send_failure(AgentFailure(_PROVIDER, message)) + raise RuntimeError(message) # Point the MCP resolver at this room's current tools for the # server-side tool calls this turn will make. @@ -407,34 +424,25 @@ async def _run_turn( "Room %s: Sending message to Letta agent %s", room_id, room_ctx.agent_id ) try: - final_text_parts = await asyncio.wait_for( - self._send_message( - agent_id=room_ctx.agent_id, - content=content, - tools=tools, - room_ctx=room_ctx, - room_id=room_id, - reply_to_sender_id=msg.sender_id, - ), - timeout=self.config.turn_timeout_s, - ) - except asyncio.TimeoutError: - logger.error( - "Room %s: Letta turn timed out after %ss", - room_id, - self.config.turn_timeout_s, - ) - await self._report_error( - tools, - f"Letta agent response timed out after {self.config.turn_timeout_s}s", + final_text_parts = await self._send_message( + agent_id=room_ctx.agent_id, + content=content, + tools=tools, + room_ctx=room_ctx, + room_id=room_id, + reply_to_sender_id=msg.sender_id, ) + except DeliveryFailedError as e: + reraise_delivery_cause(e) + except TurnResultAlreadyReported: + raise except Exception as e: logger.exception("Room %s: Error during Letta turn: %s", room_id, e) - await self._report_error(tools, str(e)) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) + raise else: - if room_ctx.pending_seed: - room_ctx.pending_seed = [] - room_ctx.last_interaction = datetime.now(timezone.utc) if final_text_parts: room_ctx.summary = self._extract_summary( final_text_parts, self.config.summary_max_length @@ -465,20 +473,38 @@ async def _send_message( turn_usage = TurnUsage() try: - # Use Conversations API in shared mode, direct agent API in per_room mode - if self.config.mode == "shared" and room_ctx.conversation_id: - conversation_stream = await self._client.conversations.messages.create( - conversation_id=room_ctx.conversation_id, - messages=messages, + try: + # turn_timeout_s bounds only the round-trip to Letta -- response + # processing (including deliver_reply, below) runs unbounded so a + # slow Band-side delivery is never mislabeled as a Letta timeout. + response_messages, turn_usage = await asyncio.wait_for( + self._call_provider(agent_id, messages, room_ctx), + timeout=self.config.turn_timeout_s, ) - response_messages = [resp_msg async for resp_msg in conversation_stream] - else: - response = await self._client.agents.messages.create( - agent_id=agent_id, - messages=messages, + room_ctx.pending_seed = [] + room_ctx.last_interaction = datetime.now(timezone.utc) + except asyncio.TimeoutError: + # Caught and reported here, at the exact call this timeout + # bounds -- a TimeoutError surfacing from anywhere else in + # this method (e.g. tool-event reporting below) is a + # genuine unrelated failure, not a Letta provider timeout, + # and must reach _run_turn's generic exception handler + # instead of being conflated with this one. + logger.error( + "Room %s: Letta turn timed out after %ss", + room_id, + self.config.turn_timeout_s, ) - response_messages = list(response.messages) - turn_usage = self._usage_from_response(response) + await tools.send_failure( + AgentFailure( + _PROVIDER, + f"Letta agent response timed out after {self.config.turn_timeout_s}s", + FAILURE_CODE_TIMEOUT, + ) + ) + raise TurnResultAlreadyReported( + "Letta provider call timed out" + ) from None return await self._process_response_messages( response_messages, @@ -490,6 +516,28 @@ async def _send_message( # No-op unless Emit.USAGE is on; best-effort, never raises. await self.emit_usage(tools, turn_usage) + async def _call_provider( + self, + agent_id: str, + messages: list[dict[str, str]], + room_ctx: RoomContext, + ) -> tuple[list[Any], TurnUsage]: + """Round-trip to the Letta API -- no response processing or delivery.""" + # Use Conversations API in shared mode, direct agent API in per_room mode + if self.config.mode == "shared" and room_ctx.conversation_id: + conversation_stream = await self._client.conversations.messages.create( + conversation_id=room_ctx.conversation_id, + messages=messages, + ) + response_messages = [resp_msg async for resp_msg in conversation_stream] + return response_messages, TurnUsage() + + response = await self._client.agents.messages.create( + agent_id=agent_id, + messages=messages, + ) + return list(response.messages), self._usage_from_response(response) + async def _process_response_messages( self, response_messages: list[Any], @@ -576,11 +624,12 @@ async def _process_response_messages( room_id, self._mcp.send_message_tool, ) - await self._report_error( - tools, + detail = ( f"Letta agent did not call {self._mcp.send_message_tool} " - "(auto-relay disabled); its reply was dropped", + "(auto-relay disabled); its reply was dropped" ) + await tools.send_failure(AgentFailure(_PROVIDER, detail)) + raise TurnResultAlreadyReported(detail) else: final_text = "\n\n".join(final_text_parts) mentions = [reply_to_sender_id] if reply_to_sender_id else None @@ -589,7 +638,7 @@ async def _process_response_messages( room_id, self._mcp.send_message_tool, ) - await tools.send_message(final_text, mentions=mentions) + await deliver_reply(tools, final_text, mentions=mentions) return final_text_parts @@ -1174,10 +1223,3 @@ def _extract_summary(parts: list[str], max_length: int = 150) -> str: if len(text) <= max_length: return text return text[:max_length].rsplit(" ", 1)[0] + "..." - - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - """Send error event (best effort).""" - try: - await tools.send_event(content=f"Error: {error}", message_type="error") - except Exception: - logger.debug("Failed to report error to platform: %s", error) diff --git a/src/band/adapters/opencode/adapter.py b/src/band/adapters/opencode/adapter.py index 961345ba8..974b6ce6f 100644 --- a/src/band/adapters/opencode/adapter.py +++ b/src/band/adapters/opencode/adapter.py @@ -13,13 +13,18 @@ from typing import ClassVar, Any import httpx +from band_sdk_core import AgentFailure from typing_extensions import Unpack from band.adapters.opencode.approvals import ApprovalPorts, RoomApprovals from band.adapters.opencode.config import OpencodeAdapterConfig from band.converters.opencode import OpencodeHistoryConverter from band.core.exceptions import BandConnectionError -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import ( + FAILURE_CODE_TIMEOUT, + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, +) from band.core.simple_adapter import SimpleAdapter from band.core.types import ( AdapterFeatures, @@ -63,6 +68,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "opencode" + _OPENCODE_SYSTEM_NOTE = """\ Responses are relayed back into the Band room by the adapter. Use the band_ prefixed tools (e.g. band_send_message) for Band platform actions when available. @@ -493,16 +500,20 @@ async def on_message( raise except httpx.HTTPStatusError as exc: logger.exception("OpenCode request failed for room %s", room_id) - await tools.send_event( - self._format_http_error(exc), - "error", + await tools.send_failure( + AgentFailure( + _PROVIDER, + self._format_http_error(exc), + str(exc.response.status_code), + ) ) + raise except Exception: logger.exception("Unexpected OpenCode adapter failure in room %s", room_id) - await tools.send_event( - "OpenCode failed while processing the message.", - "error", + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) ) + raise async def on_cleanup(self, room_id: str) -> None: room_state: RoomState | None = None @@ -931,9 +942,12 @@ async def _watch_turn_completion( ) await self._abort_session(room_state, "timed-out") if room_state.tools: - await room_state.tools.send_event( - "OpenCode timed out before completing the turn.", - "error", + await room_state.tools.send_failure( + AgentFailure( + _PROVIDER, + "OpenCode timed out before completing the turn.", + FAILURE_CODE_TIMEOUT, + ) ) # Tokens spent before the timeout were still spent — emit them, same # as the success path (best-effort; no-op if none captured). @@ -1127,8 +1141,8 @@ async def _deliver_fallback_text(self, room_state: RoomState) -> None: text, mentions=room_state.pending_mentions ) elif room_state.last_error_message: - await room_state.tools.send_event( - room_state.last_error_message, "error" + await room_state.tools.send_failure( + AgentFailure(_PROVIDER, room_state.last_error_message) ) elif not replied: await room_state.tools.send_message( diff --git a/src/band/adapters/parlant.py b/src/band/adapters/parlant.py index b6490c274..f56e13793 100644 --- a/src/band/adapters/parlant.py +++ b/src/band/adapters/parlant.py @@ -14,9 +14,15 @@ from dataclasses import dataclass, field from typing import ClassVar, TYPE_CHECKING, Any +from band_sdk_core import AgentFailure from typing_extensions import Unpack -from band.core.protocols import AgentToolsProtocol +from band.core.delivery import ( + DeliveryFailedError, + deliver_reply, + reraise_delivery_cause, +) +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE, AgentToolsProtocol from band.core.simple_adapter import SimpleAdapter from band.core.types import Capability, Emit, FeatureKwargs, PlatformMessage from band.integrations.parlant.server import running_parlant_server @@ -36,6 +42,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "parlant" + # Parlant preamble message tag - used to identify acknowledgment messages before tool execution PARLANT_PREAMBLE_TAG = "__preamble__" @@ -376,8 +384,10 @@ async def on_message( logger.debug("Handling message %s in room %s", msg.id, room_id) if not self._app: - logger.error("Parlant Application not initialized") - return + message = "Parlant Application not initialized" + logger.error(message) + await tools.send_failure(AgentFailure(_PROVIDER, message)) + raise RuntimeError(message) app = self._app sender_name = msg.sender_name or msg.sender_id or "User" @@ -387,8 +397,10 @@ async def on_message( session_id = await self._get_or_create_session(room_id, sender_name) except Exception as e: logger.error("Failed to get/create session for room %s: %s", room_id, e) - await self._report_error(tools, f"Session initialization failed: {e}") - return + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) + raise session_id_str = str(session_id) # Set tools for this session (keyed by session_id for cross-task access) @@ -443,9 +455,13 @@ async def on_message( sender_name=sender_name, ) + except DeliveryFailedError as e: + reraise_delivery_cause(e) except Exception as e: logger.error("Error processing message: %s", e, exc_info=True) - await self._report_error(tools, str(e)) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) raise finally: # Clear tools after message processing @@ -791,18 +807,10 @@ async def _process_agent_response( room_id, message_content[:100], ) - try: - await tools.send_message( - message_content, mentions=[sender_name] - ) - logger.info("Room %s: Message sent successfully", room_id) - except Exception as e: - logger.error( - "Room %s: Error sending message: %s", - room_id, - e, - exc_info=True, - ) + await deliver_reply( + tools, message_content, mentions=[sender_name] + ) + logger.info("Room %s: Message sent successfully", room_id) else: logger.warning( "Room %s: Empty message content in event", @@ -854,13 +862,6 @@ async def on_cleanup(self, room_id: str) -> None: logger.debug("Room %s: Cleaned up Parlant session", room_id) - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - """Send error event (best effort).""" - try: - await tools.send_event(content=f"Error: {error}", message_type="error") - except Exception: - logger.exception("Failed to send error event") - async def cleanup_all(self) -> None: """Release all sessions and the owned Parlant server (call on stop).""" self._room_sessions.clear() diff --git a/src/band/adapters/pydantic_ai.py b/src/band/adapters/pydantic_ai.py index d48ab550c..68e1b52b4 100644 --- a/src/band/adapters/pydantic_ai.py +++ b/src/band/adapters/pydantic_ai.py @@ -12,7 +12,7 @@ from collections.abc import Callable from typing import Any, ClassVar, Literal, cast, get_origin, get_type_hints -import httpx +from band_sdk_core import AgentFailure from pydantic_ai import ( Agent, AgentRunResultEvent, @@ -35,10 +35,13 @@ ) from pydantic_ai.models import ModelRequestContext -from band_rest.core.api_error import ApiError from typing_extensions import Unpack -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, + TurnResultAlreadyReported, +) from band.core.simple_adapter import SimpleAdapter from band.core.task_types import TaskAssignmentStatus, TaskLifecycleState, TaskListState from band.core.types import ( @@ -74,6 +77,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "pydantic_ai" + OUTPUT_RETRIES_EXHAUSTED = "exceeded maximum output retries" """pydantic-ai's wording when a run burns its output-retry budget. @@ -1025,18 +1030,25 @@ async def on_message( room_id, dropped, ) - except UnexpectedModelBehavior as e: + except Exception as e: # A turn that already did its work must not fail over the reply the model # owes pydantic-ai. Allowing `None` — and normalizing blank text into it # — ends the ordinary nothing-left-to-say response cleanly, but some # other response the run cannot turn into output can still spend the # refused output budget. Once a terminal tool has run (a # band_send_message reply, a band_store_memory, ...) the work already went - # out, so that exhaustion is benign — swallow it. Genuine no-response - # failures (no terminal tool ran — only read-only lookups or failed - # tools) still propagate here, unlike the crewai adapter, which cannot - # tell them apart from the empty completion that ends its every turn. - if tool_executed and _is_output_retries_exhausted(e): + # out, so that exhaustion is benign — swallow it. Every other exception — + # a different UnexpectedModelBehavior, or any other type now that this + # catches broadly for send_failure reporting — still surfaces and + # propagates. Unlike the crewai adapter, which cannot tell a genuine + # failure apart from the empty completion that ends its every turn, + # pydantic-ai raises the exhausted-retries case as its own distinct type, + # so the isinstance check (not just the message match) is load-bearing. + if ( + tool_executed + and isinstance(e, UnexpectedModelBehavior) + and _is_output_retries_exhausted(e) + ): logger.warning( "Room %s: Pydantic AI exhausted its output retries after " "the agent already did productive work this turn; treating as " @@ -1056,6 +1068,10 @@ async def on_message( ModelRequest(parts=[UserPromptPart(content=user_message)]), ] return + logger.exception("Room %s: Pydantic AI turn failed", room_id) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) raise finally: capture_cm.__exit__(None, None, None) @@ -1075,7 +1091,12 @@ async def on_message( # either answered in plain text or said nothing at all. Surface it as an # error (mirrors the crewai adapter) instead of letting it vanish. if not tool_executed: - await self._report_error(tools, missing_reply_error("Pydantic AI")) + logger.warning( + "Room %s: Pydantic AI turn produced nothing for the room", room_id + ) + detail = missing_reply_error("Pydantic AI") + await tools.send_failure(AgentFailure(_PROVIDER, detail)) + raise TurnResultAlreadyReported(detail) logger.debug( "Room %s: Pydantic AI agent completed (history now has %s messages)", @@ -1151,18 +1172,6 @@ def _usage_from_messages(messages: list[ModelMessage]) -> TurnUsage: total = total + PydanticAIAdapter._usage_from_usage_obj(message.usage) return total - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - """Send an error event to the room (best effort). - - Structurally mirrors the crewai adapter, but narrows the catch to the REST - call's real failure modes (ApiError = HTTP status, httpx = transport) so a - failed error-report never crashes the turn — while a real bug still raises. - """ - try: - await tools.send_event(content=f"Error: {error}", message_type="error") - except (ApiError, httpx.HTTPError) as e: - logger.warning("Failed to send error event: %s", e) - # --- Copied from BandPydanticAgent._cleanup_session --- async def on_cleanup(self, room_id: str) -> None: """Clean up message history when agent leaves a room.""" diff --git a/src/band/adapters/strands.py b/src/band/adapters/strands.py index 95ce084fc..f221c1eeb 100644 --- a/src/band/adapters/strands.py +++ b/src/band/adapters/strands.py @@ -7,7 +7,7 @@ from collections.abc import Awaitable, Callable, Mapping from typing import Any, ClassVar, cast -import httpx +from band_sdk_core import AgentFailure from pydantic import BaseModel try: @@ -31,10 +31,13 @@ "Install with: uv add band-sdk[strands]" ) from error -from band_rest.core.api_error import ApiError from typing_extensions import Unpack -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, + TurnResultAlreadyReported, +) from band.core.simple_adapter import SimpleAdapter from band.core.tool_filter import filter_tool_schemas from band.core.types import ( @@ -73,6 +76,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "strands" + def _format_tool_output(value: object) -> str: """Return a stable text representation accepted by Strands tool results.""" @@ -525,12 +530,20 @@ async def _run_turn( hooks: BandTurnHooks, ) -> None: """Run the framework loop while preserving transcript and usage on failure.""" - agent = self._build_agent(history, tools, hooks) + agent: Agent | None = None try: + agent = self._build_agent(history, tools, hooks) await agent.invoke_async(message) + except Exception: + logger.exception("Room %s: Strands turn failed", room_id) + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + ) + raise finally: - self._message_history[room_id] = agent.messages - await self.emit_usage(tools, self._usage_from_agent(agent)) + if agent is not None: + self._message_history[room_id] = agent.messages + await self.emit_usage(tools, self._usage_from_agent(agent)) async def on_message( self, @@ -570,7 +583,12 @@ async def on_message( hooks=hooks, ) if not hooks.terminal_fired: - await self._report_error(tools, missing_reply_error("Strands")) + logger.warning( + "Room %s: Strands turn produced nothing for the room", room_id + ) + detail = missing_reply_error("Strands") + await tools.send_failure(AgentFailure(_PROVIDER, detail)) + raise TurnResultAlreadyReported(detail) logger.debug( "Room %s: Strands agent completed (history now has %s messages)", room_id, @@ -592,16 +610,6 @@ def _usage_from_agent(agent: Agent) -> TurnUsage: cache_write="cacheWriteInputTokens", ) - async def _report_error(self, tools: AgentToolsProtocol, error: str) -> None: - """Post a best-effort room-visible adapter error.""" - try: - await tools.send_event( - content=f"Error: {error}", - message_type=MessageType.ERROR, - ) - except (ApiError, httpx.HTTPError) as report_error: - logger.warning("Failed to send error event: %s", report_error) - async def on_cleanup(self, room_id: str) -> None: """Discard the transcript when Band removes the adapter from a room.""" if self._message_history.pop(room_id, None) is not None: diff --git a/src/band/core/delivery.py b/src/band/core/delivery.py new file mode 100644 index 000000000..6a8427869 --- /dev/null +++ b/src/band/core/delivery.py @@ -0,0 +1,52 @@ +"""Delivery-vs-provider-failure misclassification guard. + +An adapter's reply/bookkeeping post to the room (``send_message``) is Band-side +delivery, never a provider failure -- even when the ``send_message`` call sits +inside a try/except that also handles real provider errors. ``deliver_reply`` +wraps the cause in ``DeliveryFailedError`` so that shared except block can tell +the two apart and re-raise the original cause before its provider branch. +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING, Any, NoReturn + +if TYPE_CHECKING: + from band.core.protocols import AgentToolsProtocol + +logger = logging.getLogger(__name__) + + +class DeliveryFailedError(Exception): + """Wraps a ``send_message`` failure so it is never mistaken for a + provider failure by a shared except block.""" + + def __init__(self, cause: BaseException) -> None: + super().__init__(str(cause)) + self.cause = cause + + +def reraise_delivery_cause(e: DeliveryFailedError) -> NoReturn: + """Log then re-raise a ``DeliveryFailedError``'s cause. + + Band-side reply delivery failed, never a provider failure -- re-raises + the cause (not this wrapper) so mark_failed/retry bookkeeping keys off + the real exception. + """ + logger.exception("Reply delivery failed: %s", e.cause) + raise e.cause from None + + +async def deliver_reply( + tools: "AgentToolsProtocol", + content: str, + mentions: list[str] | list[dict[str, str]] | None = None, +) -> Any: + """Send a reply, raising ``DeliveryFailedError`` on failure instead of + the raw exception, so the caller's except can distinguish a delivery + failure from a provider failure.""" + try: + return await tools.send_message(content, mentions=mentions) + except Exception as exc: + raise DeliveryFailedError(exc) from exc diff --git a/src/band/core/protocols.py b/src/band/core/protocols.py index 9373fb4bd..66f1634fb 100644 --- a/src/band/core/protocols.py +++ b/src/band/core/protocols.py @@ -2,8 +2,15 @@ from __future__ import annotations +import logging from typing import TYPE_CHECKING, Any, Literal, Protocol, TypeVar, runtime_checkable +from band_sdk_core import AgentFailure + +from band.core.content import has_visible_content + +logger = logging.getLogger(__name__) + if TYPE_CHECKING: from anthropic.types import ToolParam @@ -27,6 +34,73 @@ T = TypeVar("T") +# Shared ``AgentFailure.code`` value for a stalled/unresponsive provider turn, +# so every adapter's timeout branch reports the same code instead of each +# retyping the literal. +FAILURE_CODE_TIMEOUT = "timeout" + +# Shared generic message for a caught provider exception whose text must not +# reach the room (it can embed DB strings, paths, or tokens) -- the full +# detail goes to the agent log via logger.exception instead. +GENERIC_PROVIDER_FAILURE_MESSAGE = ( + "Internal error while processing message; see agent logs." +) + + +class TurnResultAlreadyReported(Exception): + """A terminal turn failure that a nested handler already reported via + ``send_failure``. An adapter's outer ``except`` re-raises this without + reporting the same failure a second time.""" + + +def to_failure_event(failure: AgentFailure) -> tuple[str, dict[str, Any]]: + """Shared shape every ``send_failure`` implementation posts as an `error` event. + + A provider message can arrive blank (``Exception()`` and ``str("")`` both + reach here empty). The platform rejects a blank chat event, so an + unguarded blank message would make the failure vanish from the room + entirely. The fallback string is part of the TS/Python parity contract — + it must match ``toFailureEvent``'s exactly. + """ + content = ( + failure.message.strip() + if has_visible_content(failure.message) + else f"{failure.provider} failed without an error message." + ) + return content, {"failure": failure.to_dict()} + + +async def send_event_safe( + tools: "AgentToolsProtocol", + content: str, + message_type: str, + metadata: dict[str, Any] | None = None, + *, + log_label: str | None = None, + log_level: int = logging.WARNING, +) -> bool: + """Send a best-effort platform event, logging instead of raising on failure. + + For events whose loss is tolerable (a thought, a lifecycle/task marker), + unlike a ``send_message`` call a caller depends on as a control signal. + Returns whether the event was actually accepted, so a caller that only + wants to update its own bookkeeping once delivery is confirmed (e.g. + marking a session id persisted) can act on it. + """ + try: + await tools.send_event( + content=content, message_type=message_type, metadata=metadata + ) + except Exception: + logger.log( + log_level, + "Failed to send %s event", + log_label or message_type, + exc_info=True, + ) + return False + return True + @runtime_checkable class HistoryConverter(Protocol[T]): @@ -79,6 +153,16 @@ async def send_event( """Send an event (tool_call, tool_result, thought, error, task).""" ... + async def send_failure(self, failure: AgentFailure) -> Any: + """Report a provider-originated failure as a structured `error` event. + + Best-effort: swallows its own reporting failure rather than raising, + so a caller reporting a failure never has that report replaced by an + unrelated exception. Unlike ``send_event``, whose raising callers + depend on it as a control signal. + """ + ... + async def add_participant(self, identifier: str, role: str = "member") -> Any: """Add a participant to the current room by handle, name, or ID.""" ... diff --git a/src/band/integrations/a2a/adapter.py b/src/band/integrations/a2a/adapter.py index 15042ac84..f9c257b8e 100644 --- a/src/band/integrations/a2a/adapter.py +++ b/src/band/integrations/a2a/adapter.py @@ -19,15 +19,27 @@ Task, TaskState, ) +from band_sdk_core import AgentFailure from typing_extensions import Unpack from band.converters.a2a import A2AHistoryConverter -from band.core.protocols import AgentToolsProtocol +from band.core.delivery import ( + DeliveryFailedError, + deliver_reply, + reraise_delivery_cause, +) +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, + TurnResultAlreadyReported, + send_event_safe, +) from band.core.simple_adapter import SimpleAdapter from band.core.types import Capability, Emit, FeatureKwargs, PlatformMessage from band.integrations.a2a.protocol import ( TERMINAL_TASK_STATE_NAMES, TERMINAL_TASK_STATES, + RETRYABLE_TASK_FAILURE_STATES, apply_task_stream_event, state_name, task_id_from_stream_event, @@ -37,6 +49,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "a2a" + # httpx's read timeout resets on every chunk received, so this bounds the gap # between SSE events, not the turn as a whole. Generous enough for the # multi-second silences of a live LLM call or tool loop; still finite, so a @@ -172,13 +186,16 @@ async def on_message( event, tools, room_id, msg.sender_id, msg.sender_name ) + except DeliveryFailedError as e: + reraise_delivery_cause(e) + except TurnResultAlreadyReported: + raise except Exception as e: logger.exception("A2A agent error: %s", e) - await tools.send_event( - content=f"A2A agent error: {e}", - message_type="error", - metadata={"a2a_error": str(e)}, + await tools.send_failure( + AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) ) + raise async def _handle_event( self, @@ -208,8 +225,19 @@ async def _handle_event( finally: # A terminal task must be persisted and released even when Band # delivery fails, or the room keeps addressing a finished task. + # Best-effort: a failure here must never replace an exception + # already propagating from the try block above, or a Band + # delivery outage gets misreported as a fabricated provider + # failure once it reaches on_message's except clauses. if state in TERMINAL_TASK_STATES: - await self._emit_task_event(tools, task, state) + try: + await self._emit_task_event(tools, task, state) + except Exception: + logger.exception( + "Failed to emit terminal task event (room=%s, task=%s)", + room_id, + task.id, + ) self._finalize_task(room_id, task.id) async def _deliver_message( @@ -222,8 +250,9 @@ async def _deliver_message( """Forward a direct A2A message to its Band sender.""" text = get_message_text(message) if text: - await tools.send_message( - content=text, + await deliver_reply( + tools, + text, mentions=[{"id": sender_id, "name": sender_name or ""}], ) @@ -254,27 +283,29 @@ async def _deliver_task_update( if state == TaskState.TASK_STATE_WORKING: status_text = self._get_status_text(task) if status_text: - await tools.send_event(content=status_text, message_type="thought") + await send_event_safe(tools, status_text, "thought") return if state == TaskState.TASK_STATE_INPUT_REQUIRED: text = self._get_status_text(task) or "Please provide more information." - await tools.send_message(content=text, mentions=[sender]) + await deliver_reply(tools, text, mentions=[sender]) return if state == TaskState.TASK_STATE_COMPLETED: response = self._extract_response(task) if response: - await tools.send_message(content=response, mentions=[sender]) + await deliver_reply(tools, response, mentions=[sender]) return if state in TERMINAL_TASK_STATES: - error_text = self._get_status_text(task) or f"Task {state_name(state)}" - await tools.send_event( - content=error_text, - message_type="error", - metadata={"a2a_state": state_name(state)}, + state_str = state_name(state) + error_text = self._get_status_text(task) or f"Task {state_str}" + logger.warning( + "Task %s: peer A2A task ended in state %s", task.id, state_str ) + await tools.send_failure(AgentFailure(_PROVIDER, error_text, state_str)) + if state in RETRYABLE_TASK_FAILURE_STATES: + raise TurnResultAlreadyReported(error_text) def _finalize_task(self, room_id: str, task_id: str) -> None: """Release a terminal task after its Band output and state are persisted.""" diff --git a/src/band/integrations/a2a/gateway/adapter.py b/src/band/integrations/a2a/gateway/adapter.py index dbb00f5f2..2247525ac 100644 --- a/src/band/integrations/a2a/gateway/adapter.py +++ b/src/band/integrations/a2a/gateway/adapter.py @@ -9,12 +9,13 @@ import re from contextlib import asynccontextmanager from collections.abc import AsyncIterator -from typing import ClassVar +from typing import Any, ClassVar from uuid import uuid4 from a2a.server.agent_execution import AgentExecutor, RequestContext from a2a.server.events import EventQueue from a2a.types import Task, TaskState, TaskStatus +from band_sdk_core import AgentFailure from typing_extensions import Unpack from band.client.rest import ( @@ -28,7 +29,7 @@ ) from band.converters.a2a_gateway import GatewayHistoryConverter from band.core.content import BLANK_CONTENT_ERROR -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import FAILURE_CODE_TIMEOUT, AgentToolsProtocol from band.core.simple_adapter import SimpleAdapter from band.core.types import Capability, Emit, FeatureKwargs, PlatformMessage from band.platform.posting import post_event, post_message @@ -40,6 +41,8 @@ logger = logging.getLogger(__name__) +_PROVIDER = "a2a-gateway" + @dataclass class GatewayRequest: @@ -79,6 +82,56 @@ def slugify(name: str) -> str: return slug.strip("-") # Remove leading/trailing dashes +_GATEWAY_ERROR_MAX_CHARS = 240 +_BEARER_TOKEN_RE = re.compile(r"Bearer\s+[^\s,;]+", re.IGNORECASE) +# The value group excludes only "," and ";" (not whitespace) so a +# scheme-prefixed credential (e.g. "Authorization: ApiKey sk-...") gets +# redacted in full instead of leaking everything past the first space. +_CREDENTIAL_KV_RE = re.compile( + r"(token|authorization|api[_-]?key|access[_-]?key|secret|password)" + r"\s*[:=]\s*[^,;]+", + re.IGNORECASE, +) + + +def _redact_credentials(text: str) -> str: + """Redact bearer tokens/API keys a message may embed.""" + redacted = _BEARER_TOKEN_RE.sub("Bearer [REDACTED]", text) + return _CREDENTIAL_KV_RE.sub(r"\1=[REDACTED]", redacted) + + +def _redact_credentials_deep(value: Any) -> Any: + """Recursively redact credentials from a peer's ``AgentFailure.detail``. + + ``detail`` is untrusted, adapter-defined structure (e.g. Codex's own + ``codex_additional_details`` echoes upstream error text) that can nest + a credential-bearing string at any depth before it reaches an external + A2A client. + """ + if isinstance(value, str): + return _redact_credentials(value) + if isinstance(value, dict): + return {key: _redact_credentials_deep(item) for key, item in value.items()} + if isinstance(value, list): + return [_redact_credentials_deep(item) for item in value] + return value + + +def _sanitize_gateway_error_message(exc: BaseException) -> str: + """Redact bearer tokens/API keys before an internal exception message + reaches an external A2A client, and cap its length. + + Mirrors the TS SDK's ``sanitizeGatewayErrorMessage``. + """ + trimmed = str(exc).strip() + if not trimmed: + return "Unknown error" + redacted = _redact_credentials(trimmed) + if len(redacted) <= _GATEWAY_ERROR_MAX_CHARS: + return redacted + return f"{redacted[: _GATEWAY_ERROR_MAX_CHARS - 3]}..." + + class A2AGatewayAdapter(SimpleAdapter[GatewaySessionState]): """Gateway adapter exposing Band peers as A2A endpoints. @@ -339,14 +392,19 @@ async def _execute_a2a( request.pending.task.id, ) raise - except Exception: + except Exception as exc: logger.exception( "A2A request failed: room=%s context=%s task=%s", request.room_id, request.context_id, request.pending.task.id, ) - await request.pending.fail("A2A request failed") + failure = AgentFailure( + _PROVIDER, + _sanitize_gateway_error_message(exc), + type(exc).__name__, + ) + await request.pending.fail("A2A request failed", failure=failure.to_dict()) raise else: if completed: @@ -427,7 +485,12 @@ async def _await_response(self, request: GatewayRequest) -> bool: request.pending.task.id, self.config.response_timeout_s, ) - await request.pending.fail("Timed out waiting for a Band response") + failure = AgentFailure( + _PROVIDER, + "Timed out waiting for a Band response", + FAILURE_CODE_TIMEOUT, + ) + await request.pending.fail(failure.message, failure=failure.to_dict()) return False return True @@ -554,7 +617,19 @@ async def _publish_band_response( ) -> None: """Translate Band's message category into an A2A task intent.""" if msg.message_type == "error": - await pending.fail(msg.content) + # The peer's own adapter already built this AgentFailure (see + # to_failure_event) -- relay it rather than re-tagging its + # provider as "a2a-gateway", but still redact credentials the + # peer's own message may embed before it reaches an external + # A2A client, same as this gateway's own exception path. + failure = ( + msg.metadata.get("failure") if isinstance(msg.metadata, dict) else None + ) + if isinstance(failure, dict): + failure = _redact_credentials_deep(failure) + else: + failure = None + await pending.fail(_redact_credentials(msg.content), failure=failure) elif msg.message_type in ("thought", "tool_call", "tool_result"): await pending.report_progress(msg.content) else: diff --git a/src/band/integrations/a2a/gateway/types.py b/src/band/integrations/a2a/gateway/types.py index 5ed3e848b..3f0f83efe 100644 --- a/src/band/integrations/a2a/gateway/types.py +++ b/src/band/integrations/a2a/gateway/types.py @@ -4,11 +4,12 @@ import asyncio from dataclasses import dataclass, field +from typing import Any from a2a.server.events import EventQueue from a2a.server.tasks import TaskUpdater from a2a.helpers import new_text_message -from a2a.types import Message, Task +from a2a.types import Message, Task, TaskState @dataclass @@ -67,12 +68,22 @@ async def complete_with_message(self, content: str) -> None: await self._updater.complete(self._message(content)) self.done.set() - async def fail(self, reason: str) -> None: - """Publish a terminal failure and release the request.""" + async def fail(self, reason: str, *, failure: dict[str, Any] | None = None) -> None: + """Publish a terminal failure and release the request. + + ``failure`` is the wire-shape ``AgentFailure`` dict (see + ``to_failure_event``), attached as task status metadata alongside + ``reason``'s freeform text so an A2A client can recover structured + provider-failure detail. + """ async with self._lock: if self.done.is_set(): return - await self._updater.failed(self._message(reason)) + await self._updater.update_status( + TaskState.TASK_STATE_FAILED, + message=self._message(reason), + metadata={"failure": failure} if failure else None, + ) self.done.set() async def cancel(self) -> None: diff --git a/src/band/integrations/a2a/protocol.py b/src/band/integrations/a2a/protocol.py index fa191cec7..ede46d5da 100644 --- a/src/band/integrations/a2a/protocol.py +++ b/src/band/integrations/a2a/protocol.py @@ -17,6 +17,13 @@ } ) +RETRYABLE_TASK_FAILURE_STATES = frozenset( + { + TaskState.TASK_STATE_FAILED, + TaskState.TASK_STATE_REJECTED, + } +) + # Terminal states as persisted in task-event metadata. Includes the values # written by the pre-protobuf adapter (a2a-sdk 0.x string enums), so rooms # with history from before the migration still rehydrate as terminal. diff --git a/src/band/integrations/acp/client_adapter.py b/src/band/integrations/acp/client_adapter.py index 57666a0e3..2023dd9e1 100644 --- a/src/band/integrations/acp/client_adapter.py +++ b/src/band/integrations/acp/client_adapter.py @@ -7,16 +7,24 @@ import os import shutil from collections.abc import Callable +from contextlib import suppress from typing import Any, ClassVar from uuid import uuid4 from acp import spawn_agent_process +from acp.exceptions import RequestError from acp.schema import HttpMcpServer, SseMcpServer +from band_sdk_core import AgentFailure from typing_extensions import Unpack from band.converters.acp_client import ACPClientHistoryConverter from band.converters.helpers import build_replay_messages -from band.core.protocols import AgentToolsProtocol +from band.core.delivery import DeliveryFailedError, reraise_delivery_cause +from band.core.protocols import ( + FAILURE_CODE_TIMEOUT, + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, +) from band.core.simple_adapter import SimpleAdapter from band.core.types import ( AdapterFeatures, @@ -60,6 +68,13 @@ logger = logging.getLogger(__name__) +_PROVIDER = "acp" + + +class ACPTurnTimeoutError(TimeoutError): + """The adapter deadline expired before the ACP prompt completed.""" + + LocalMcpServerConfig = HttpMcpServer | SseMcpServer # Prefixes the change-triggered roster/contacts updates injected into a @@ -125,6 +140,18 @@ def _resolve_launcher(command: list[str]) -> list[str]: return [resolved, *command[1:]] if resolved else list(command) +def _to_agent_failure(exc: Exception) -> AgentFailure: + """Parse a turn-ending exception into the shared provider-failure shape. + + ``RequestError`` is raised for a JSON-RPC error the remote agent + returned; its numeric ``code``/``data`` carry more than the generic + message alone. + """ + if isinstance(exc, RequestError): + return AgentFailure(_PROVIDER, str(exc), str(exc.code), exc.data) + return AgentFailure(_PROVIDER, GENERIC_PROVIDER_FAILURE_MESSAGE) + + class ACPClientAdapter(SimpleAdapter[ACPClientSessionState]): """Adapter that forwards Band messages to a remote ACP agent. @@ -157,6 +184,7 @@ def __init__( port: int | None = None, custom_section: str = "", spawn_process: SpawnProcess | None = None, + turn_timeout_s: float = 300.0, **features: Unpack[FeatureKwargs], ) -> None: super().__init__( @@ -174,6 +202,7 @@ def __init__( self._auth_method = auth_method self._profile = profile self._custom_section = custom_section + self._turn_timeout_s = turn_timeout_s self._runtime = self._build_runtime(spawn_process) self._room_to_session: dict[str, str] = {} @@ -352,19 +381,68 @@ async def on_message( session_id, self._make_permission_handler(emitter, room_id), ) - await self._runtime.prompt( - session_id=session_id, - prompt_text=prompt_text, - on_chunk=emitter.emit, + prompt_task = asyncio.create_task( + self._runtime.prompt( + session_id=session_id, + prompt_text=prompt_text, + on_chunk=emitter.emit, + ) + ) + done, _ = await asyncio.wait( + {prompt_task}, timeout=self._turn_timeout_s ) + if not done: + prompt_task.cancel() + with suppress(asyncio.CancelledError): + await prompt_task + await self._handle_turn_timeout( + room_id=room_id, session_id=session_id, tools=tools + ) + raise ACPTurnTimeoutError( + f"ACP turn timed out after {self._turn_timeout_s}s" + ) from None + await prompt_task + except DeliveryFailedError as e: + # The turn's reply is what failed to post -- Band-side delivery, + # never an ACP provider failure, so the connection stays up. + reraise_delivery_cause(e) + except ACPTurnTimeoutError: + raise except Exception as e: logger.exception("ACP agent error: %s", e) + if isinstance(e, RequestError): + await self.on_cleanup(room_id) + else: + await self.stop() + await tools.send_failure(_to_agent_failure(e)) + raise + + async def _handle_turn_timeout( + self, *, room_id: str, session_id: str, tools: AgentToolsProtocol + ) -> None: + """Cancel and report a prompt that exceeded the adapter timeout.""" + logger.error( + "ACP turn timed out after %ss (room=%s, session=%s)", + self._turn_timeout_s, + room_id, + session_id, + ) + try: + await self._runtime.cancel_turn(session_id) + except ConnectionError: await self.stop() - await tools.send_event( - content=f"ACP agent error: {e}", - message_type="error", - metadata={"acp_error": str(e)}, + except Exception: + logger.exception("ACP turn cancellation failed (room=%s)", room_id) + await self.on_cleanup(room_id) + else: + await self.on_cleanup(room_id) + await tools.send_failure( + AgentFailure( + _PROVIDER, + f"ACP agent response timed out after {self._turn_timeout_s}s", + FAILURE_CODE_TIMEOUT, ) + ) def _make_permission_handler( self, diff --git a/src/band/integrations/acp/client_runtime.py b/src/band/integrations/acp/client_runtime.py index a689f64d9..65333d7ea 100644 --- a/src/band/integrations/acp/client_runtime.py +++ b/src/band/integrations/acp/client_runtime.py @@ -232,6 +232,8 @@ async def load_session( async def prompt(self, *, session_id: str, prompt: list[object]) -> object: ... + async def cancel(self, session_id: str) -> None: ... + class ACPSpawnContextProtocol(Protocol): """Protocol for the spawn_agent_process async context manager.""" @@ -838,6 +840,11 @@ async def prompt( self._client.set_sink(session_id, None) return self.get_collected_chunks(session_id) + async def cancel_turn(self, session_id: str) -> None: + """Tell the agent to stop a timed-out room's prompt.""" + conn = await self.ensure_connection(can_respawn=False) + await conn.cancel(session_id) + def reset_session(self, session_id: str) -> None: if self._client is not None: self._client.reset_session(session_id) diff --git a/src/band/integrations/acp/room_emitter.py b/src/band/integrations/acp/room_emitter.py index 9806a0915..540a9e83f 100644 --- a/src/band/integrations/acp/room_emitter.py +++ b/src/band/integrations/acp/room_emitter.py @@ -4,7 +4,8 @@ import logging -from band.core.protocols import AgentToolsProtocol +from band.core.delivery import deliver_reply +from band.core.protocols import AgentToolsProtocol, send_event_safe from band.integrations.acp.types import ( ACPToolCall, ACPToolResult, @@ -173,13 +174,15 @@ async def __aexit__(self, exc_type: object, exc: object, tb: object) -> bool: # reply (and leak the agent's narration of the call). if not turn_replied_in_room(self._chunks): for text in self._pending_text: - await self._tools.send_message(content=text, mentions=self._mentions) - await self._tools.send_event( + await deliver_reply(self._tools, text, mentions=self._mentions) + await send_event_safe( + self._tools, content="ACP client session", message_type="task", metadata={ "acp_client_session_id": self._session_id, "acp_client_room_id": self._room_id, }, + log_label="ACP client session", ) return False diff --git a/src/band/integrations/codex/__init__.py b/src/band/integrations/codex/__init__.py index 9ddaa0403..dc3d96a3a 100644 --- a/src/band/integrations/codex/__init__.py +++ b/src/band/integrations/codex/__init__.py @@ -10,21 +10,21 @@ from .stdio_client import CodexStdioClient from .types import ( CODEX_APPROVAL_METHODS, - CODEX_ERROR_REMEDIATION, + CODEX_PROVIDER, ApprovalAuditEntry, CodexApprovalMethod, CodexItemType, CodexPlanStep, CodexSessionState, CodexTokenUsage, - build_structured_error_metadata, + build_agent_failure, parse_plan_steps, ) from .websocket_client import CodexWebSocketClient __all__ = [ "CODEX_APPROVAL_METHODS", - "CODEX_ERROR_REMEDIATION", + "CODEX_PROVIDER", "ApprovalAuditEntry", "CodexApprovalMethod", "CodexItemType", @@ -36,6 +36,6 @@ "CodexWebSocketClient", "OverloadRetryPolicy", "RpcEvent", - "build_structured_error_metadata", + "build_agent_failure", "parse_plan_steps", ] diff --git a/src/band/integrations/codex/types.py b/src/band/integrations/codex/types.py index 86913cd02..347e9206a 100644 --- a/src/band/integrations/codex/types.py +++ b/src/band/integrations/codex/types.py @@ -9,6 +9,8 @@ from enum import StrEnum from typing import Any +from band_sdk_core import AgentFailure + logger = logging.getLogger(__name__) @@ -59,6 +61,8 @@ class CodexApprovalMethod(StrEnum): CODEX_APPROVAL_METHODS: frozenset[CodexApprovalMethod] = frozenset(CodexApprovalMethod) +CODEX_PROVIDER = "codex" + @dataclass class CodexSessionState: @@ -77,46 +81,15 @@ def has_thread(self) -> bool: # Structured error types # --------------------------------------------------------------------------- -# Mapping from Codex error type to (human description, suggested action). -CODEX_ERROR_REMEDIATION: dict[str, tuple[str, str]] = { - "ContextWindowExceeded": ( - "Context window exceeded — the conversation is too long for the model.", - "compact_context", - ), - "UsageLimitExceeded": ( - "Usage limit exceeded — you have hit your API quota.", - "wait_or_upgrade", - ), - "HttpConnectionFailed": ( - "HTTP connection failed — could not reach the API.", - "check_connectivity", - ), - "SandboxError": ( - "Sandbox error — a sandbox policy violation occurred.", - "review_sandbox_policy", - ), - "Unauthorized": ( - "Unauthorized — authentication failed or expired.", - "re_authenticate", - ), - "BadRequest": ( - "Bad request — the input format is invalid.", - "check_input_format", - ), - "ResponseTooManyFailedAttempts": ( - "Too many failed attempts — the model could not produce a valid response.", - "retry_different_approach", - ), -} - - -def build_structured_error_metadata( + +def build_agent_failure( error_obj: dict[str, Any], *, thread_id: str | None = None, turn_id: str | None = None, -) -> tuple[str, dict[str, Any]]: - """Parse a Codex error dict and return (content, metadata) for a structured error event. + room_id: str | None = None, +) -> AgentFailure: + """Parse a Codex error dict into the shared provider-failure shape. The ``error_obj`` is typically the ``error`` field from a turn payload or an ``error`` notification. It may contain a nested ``codexErrorInfo`` dict with @@ -124,51 +97,47 @@ def build_structured_error_metadata( ``additionalDetails`` echoes upstream strings that may be attacker-controlled (e.g. error messages from a downstream HTTP target) and will be rendered by - downstream UIs. Consumers MUST treat the resulting - ``codex_additional_details`` metadata field as untrusted — escape it before - rendering as HTML/Markdown. This helper caps the length at - ``_MAX_ERROR_DETAIL_CHARS`` (2 KiB) so a hostile payload can't blow up - WebSocket frames or downstream storage. + downstream UIs. Consumers MUST treat the resulting ``codex_additional_details`` + detail field as untrusted — escape it before rendering as HTML/Markdown. This + helper caps the length at ``_MAX_ERROR_DETAIL_CHARS`` (2 KiB) so a hostile + payload can't blow up WebSocket frames or downstream storage. """ codex_info = error_obj.get("codexErrorInfo") or {} if not isinstance(codex_info, dict): codex_info = {} - error_type = codex_info.get("type") or "" - error_code = codex_info.get("code") or "" + raw_error_type = codex_info.get("type") + error_type = str(raw_error_type) if raw_error_type else None + error_code = codex_info.get("code") or None http_status = codex_info.get("httpStatus") - is_retryable = bool(codex_info.get("retryable", False)) + # A genuine passthrough of codexErrorInfo.retryable: absent means unknown, + # never defaulted to False. + is_retryable = codex_info.get("retryable") additional = error_obj.get("additionalDetails") - # Look up remediation - remediation = CODEX_ERROR_REMEDIATION.get(str(error_type)) - if remediation: - content, suggested_action = remediation - else: - raw_message = error_obj.get("message", "") - content = ( - str(raw_message) - if raw_message - else f"Codex error: {error_type or 'unknown'}" - ) - suggested_action = "" - - metadata: dict[str, Any] = { - "codex_error_type": error_type or None, - "codex_error_code": error_code or None, - "codex_http_status": http_status, - "codex_is_retryable": is_retryable, - "codex_suggested_action": suggested_action or None, - } + raw_message = error_obj.get("message", "") + message = ( + str(raw_message) if raw_message else f"Codex error: {error_type or 'unknown'}" + ) + + detail: dict[str, Any] = {} + if error_code: + detail["codex_error_code"] = error_code + if http_status is not None: + detail["codex_http_status"] = http_status + if is_retryable is not None: + detail["codex_is_retryable"] = bool(is_retryable) if thread_id: - metadata["codex_thread_id"] = thread_id + detail["codex_thread_id"] = thread_id if turn_id: - metadata["codex_turn_id"] = turn_id + detail["codex_turn_id"] = turn_id + if room_id: + detail["codex_room_id"] = room_id if additional is not None: capped = _cap_error_detail(additional) if capped is not None: - metadata["codex_additional_details"] = capped + detail["codex_additional_details"] = capped - return content, metadata + return AgentFailure(CODEX_PROVIDER, message, error_type, detail or None) def _cap_error_detail(value: Any) -> Any: diff --git a/src/band/runtime/tools/agent.py b/src/band/runtime/tools/agent.py index da29337c6..82465cec3 100644 --- a/src/band/runtime/tools/agent.py +++ b/src/band/runtime/tools/agent.py @@ -45,7 +45,7 @@ organization_scope_rejected_message, validate_subject_scope, ) -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import AgentToolsProtocol, to_failure_event from band.core.task_types import ( TaskAssignmentStatus, TaskIncludeOption, @@ -54,7 +54,7 @@ validate_include, ) from band.core.tool_filter import sanitize_tool_schema -from band.core.types import Capability +from band.core.types import Capability, MessageType from band.core.validation import at_least_one_of from band.runtime.tools.registry import ( TOOL_DEFINITIONS, @@ -442,6 +442,21 @@ async def send_event( ), ) + async def send_failure(self, failure: band_sdk_core.AgentFailure) -> Any: + """ + Report a provider-originated failure as a structured error event. + + Best-effort, unlike ``send_event``: this runs inside a caller's own + except block, where raising would replace the provider failure the + room is being told about with an unrelated reporting failure. + """ + content, metadata = to_failure_event(failure) + try: + return await self.send_event(content, MessageType.ERROR, metadata) + except Exception as exc: + logger.exception("send_failure could not post the failure event") + return {"ok": False, "error": str(exc)} + async def create_chatroom(self, task_id: str | None = None) -> str: """ Create a new chat room. diff --git a/src/band/testing/__init__.py b/src/band/testing/__init__.py index 911ce66df..a22ddbc1c 100644 --- a/src/band/testing/__init__.py +++ b/src/band/testing/__init__.py @@ -12,7 +12,11 @@ # Type-only imports for static analysis (pyrefly, mypy, etc.) if TYPE_CHECKING: - from band.testing.fake_tools import FakeAgentTools as FakeAgentTools + from band.testing.fake_tools import ( + FakeAgentTools as FakeAgentTools, + events_of_type as events_of_type, + reported_failures as reported_failures, + ) from band.testing.features import feature_kwargs as feature_kwargs from band.testing.phoenix_server import ( FakePhoenixServer as FakePhoenixServer, @@ -35,7 +39,7 @@ __all__, __getattr__ = lazy_exports( __name__, - fake_tools=["FakeAgentTools"], + fake_tools=["FakeAgentTools", "events_of_type", "reported_failures"], features=["feature_kwargs"], phoenix_server=["FakePhoenixServer", "JoinOutcome", "fake_phoenix_server"], platform=["platform_connection_stub"], diff --git a/src/band/testing/fake_tools.py b/src/band/testing/fake_tools.py index 5894ebc89..83c3f39fe 100644 --- a/src/band/testing/fake_tools.py +++ b/src/band/testing/fake_tools.py @@ -8,6 +8,8 @@ from datetime import datetime, timezone from typing import Any, Literal +import band_sdk_core + from band.client.rest import ( AgentContact, AgentMemory, @@ -34,8 +36,9 @@ ) from band.core.content import has_visible_content from band.core.exceptions import BandToolError +from band.core.protocols import to_failure_event from band.core.task_types import TaskAssignmentStatus, TaskLifecycleState, TaskListState -from band.core.types import Capability +from band.core.types import Capability, MessageType from band.runtime.tools import ( DEFAULT_FILE_CAPTION, FILE_UNAVAILABLE_MESSAGE, @@ -101,6 +104,9 @@ def __init__( self._hub_room_id = hub_room_id self.messages_sent: list[dict[str, Any]] = [] self.events_sent: list[dict[str, Any]] = [] + # Set to simulate a send_event REST rejection (e.g. proving + # send_failure swallows it while send_event itself still raises). + self.send_event_error: Exception | None = None self._participants: list[dict[str, Any]] = participants or [] self._room_context: list[dict[str, Any]] = list(room_context or []) # Seeds are validated and canonicalized at seed time (not list time), @@ -198,6 +204,8 @@ async def send_event( Same fidelity rationale as ``send_message``: the real send returns ``None`` without a request rather than letting the platform 422. """ + if self.send_event_error is not None: + raise self.send_event_error if not has_visible_content(content): return None event = { @@ -209,6 +217,16 @@ async def send_event( self.events_sent.append(event) return event + async def send_failure( + self, failure: band_sdk_core.AgentFailure + ) -> dict[str, Any] | None: + """Same best-effort delegation as ``AgentTools.send_failure``.""" + content, metadata = to_failure_event(failure) + try: + return await self.send_event(content, MessageType.ERROR, metadata) + except Exception as exc: + return {"ok": False, "error": str(exc)} + async def add_participant( self, identifier: str, role: str = "member" ) -> dict[str, Any]: @@ -711,3 +729,22 @@ def assert_no_messages_sent(self) -> None: assert not self.messages_sent, ( f"Expected no messages, but {len(self.messages_sent)} were sent" ) + + +def events_of_type(tools: FakeAgentTools, message_type: str) -> list[dict[str, Any]]: + """Events of ``message_type`` captured on ``tools.events_sent``.""" + return [e for e in tools.events_sent if e["message_type"] == message_type] + + +def reported_failures(tools: FakeAgentTools) -> list[dict[str, Any]]: + """Every ``AgentFailure`` reported via ``send_failure``, as its wire dict. + + Ignores an "error" event with no ``failure`` metadata -- a pre-existing, + not-yet-migrated ``send_event(..., "error")`` call site posts one without + the ``send_failure`` shape, and that isn't what this helper reports on. + """ + return [ + e["metadata"]["failure"] + for e in events_of_type(tools, MessageType.ERROR) + if "failure" in e["metadata"] + ] diff --git a/tests/adapters/agno/test_adapter.py b/tests/adapters/agno/test_adapter.py index 51d1837e8..888112b29 100644 --- a/tests/adapters/agno/test_adapter.py +++ b/tests/adapters/agno/test_adapter.py @@ -29,8 +29,9 @@ _bind_room_tools, _make_band_entrypoint, ) +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE from band.core.types import Capability, Emit, PlatformMessage -from band.testing import FakeAgentTools +from band.testing import FakeAgentTools, reported_failures from tests.adapters.agno.helpers import ( CapturingModel, @@ -889,14 +890,14 @@ async def test_emits_generic_error_event_and_reraises( room_id="room-A", ) - errors = [e for e in tools.events_sent if e["message_type"] == "error"] - assert len(errors) == 1 - assert ( - errors[0]["content"] - == "Internal error while processing message; see agent logs." - ) + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE # The exception text (which can carry secrets) must not leak to the room. - assert "secret-token" not in errors[0]["content"] + assert "secret-token" not in failures[0]["message"] + assert failures[0]["provider"] == "agno" + # A plain RuntimeError isn't a swallowed Agno run status -- no code. + assert failures[0]["code"] is None async def test_error_status_run_is_raised_and_reported( self, make_started_adapter, tools @@ -922,9 +923,10 @@ async def test_error_status_run_is_raised_and_reported( room_id="room-A", ) - errors = [e for e in tools.events_sent if e["message_type"] == "error"] - assert len(errors) == 1 - assert "secret-token" not in errors[0]["content"] + failures = reported_failures(tools) + assert len(failures) == 1 + assert "secret-token" not in failures[0]["message"] + assert failures[0]["code"] == RunStatus.error.value # A failed turn must not be committed to the room transcript. assert not adapter._message_history.get("room-A") @@ -951,9 +953,10 @@ async def test_streaming_error_event_is_raised_and_reported( room_id="room-A", ) - errors = [e for e in tools.events_sent if e["message_type"] == "error"] - assert len(errors) == 1 - assert "secret-token" not in errors[0]["content"] + failures = reported_failures(tools) + assert len(failures) == 1 + assert "secret-token" not in failures[0]["message"] + assert failures[0]["code"] == RunStatus.error.value assert not adapter._message_history.get("room-A") async def test_error_event_failure_does_not_mask_original( diff --git a/tests/adapters/copilot_sdk/fakes.py b/tests/adapters/copilot_sdk/fakes.py index cf496cfba..c836df096 100644 --- a/tests/adapters/copilot_sdk/fakes.py +++ b/tests/adapters/copilot_sdk/fakes.py @@ -147,10 +147,12 @@ def __init__( self, *, resume_error: Exception | None = None, + create_error: Exception | None = None, reply_content: str | None = "Hello from Copilot", turn_events: list[Any] | None = None, ): self.resume_error = resume_error + self.create_error = create_error self.reply_content = reply_content self.turn_events = turn_events or [] self.started = False @@ -168,6 +170,8 @@ async def stop(self) -> None: async def create_session( self, *, session_id: str | None = None, **kwargs: Any ) -> Any: + if self.create_error: + raise self.create_error session = FakeCopilotSession( session_id, kwargs, diff --git a/tests/adapters/copilot_sdk/test_reply.py b/tests/adapters/copilot_sdk/test_reply.py index 76f4c03c8..44536b757 100644 --- a/tests/adapters/copilot_sdk/test_reply.py +++ b/tests/adapters/copilot_sdk/test_reply.py @@ -5,7 +5,9 @@ import pytest from band.adapters.copilot_sdk import _COPILOT_SDK_AVAILABLE +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE from band.runtime.tools import CHAT_ID_FIELD_NAME, ToolCallOutcome +from band.testing import reported_failures from tests.adapters.copilot_sdk.fakes import ( FakeCopilotClient, FakeCopilotSession, @@ -77,8 +79,9 @@ async def test_session_error_raises_reports_and_evicts(self): session = client.sessions[0] assert session.aborted and session.disconnected - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] - assert error_events and "boom" in error_events[0]["content"] + failures = reported_failures(tools) + assert failures and failures[0]["provider"] == "copilot_sdk" + assert failures[0]["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE @pytest.mark.asyncio async def test_fallback_send_suppressed_when_band_send_message_fired(self): diff --git a/tests/adapters/copilot_sdk/test_turn_failure.py b/tests/adapters/copilot_sdk/test_turn_failure.py index 10dbf9f24..dd452b560 100644 --- a/tests/adapters/copilot_sdk/test_turn_failure.py +++ b/tests/adapters/copilot_sdk/test_turn_failure.py @@ -4,6 +4,7 @@ import pytest +from band.testing import reported_failures from tests.adapters.copilot_sdk.fakes import ( FakeCopilotClient, ToolSchemaFakeTools, @@ -32,8 +33,9 @@ async def test_failed_turn_aborts_and_evicts_session(self): # The stale turn is aborted on the runtime and the session dropped... assert dead.aborted assert dead.disconnected - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] - assert error_events + failures = reported_failures(tools) + assert failures + assert failures[0]["provider"] == "copilot_sdk" # ...so the next message starts clean, resuming by the stored id. await run_message(adapter, tools, is_session_bootstrap=False) @@ -70,3 +72,18 @@ async def test_empty_final_text_raises_no_reply(self): await run_message(adapter, tools) assert not tools.messages_sent + + @pytest.mark.asyncio + async def test_session_creation_failure_is_reported(self): + """A create_session failure, raised before on_message's own turn + processing begins, must still be reported.""" + client = FakeCopilotClient(create_error=RuntimeError("Copilot CLI unreachable")) + adapter = await make_started_adapter(client) + tools = ToolSchemaFakeTools() + + with pytest.raises(RuntimeError, match="Copilot CLI unreachable"): + await run_message(adapter, tools) + + failures = reported_failures(tools) + assert failures + assert failures[0]["provider"] == "copilot_sdk" diff --git a/tests/adapters/langgraph/conftest.py b/tests/adapters/langgraph/conftest.py index fcc041c65..7544ff435 100644 --- a/tests/adapters/langgraph/conftest.py +++ b/tests/adapters/langgraph/conftest.py @@ -30,6 +30,7 @@ def mock_tools(): tools = MagicMock() tools.send_message = AsyncMock(return_value={"status": "sent"}) tools.send_event = AsyncMock(return_value={"status": "sent"}) + tools.send_failure = AsyncMock(return_value={"status": "sent"}) tools.add_participant = AsyncMock(return_value={"id": "user-1"}) tools.remove_participant = AsyncMock(return_value={"status": "removed"}) tools.lookup_peers = AsyncMock(return_value={"peers": []}) diff --git a/tests/adapters/langgraph/test_lifecycle.py b/tests/adapters/langgraph/test_lifecycle.py index dd338ec26..27827ccf7 100644 --- a/tests/adapters/langgraph/test_lifecycle.py +++ b/tests/adapters/langgraph/test_lifecycle.py @@ -327,11 +327,45 @@ async def failing_stream(*args, **kwargs): room_id="room-123", ) - # Should have tried to report an error event, AND that event - # must NOT include the raw exception text (it can carry DB - # strings, paths, tokens, etc.). The full traceback only goes - # to the agent log via logger.exception. - mock_tools.send_event.assert_awaited() - call_kwargs = mock_tools.send_event.call_args.kwargs - assert call_kwargs["message_type"] == "error" - assert "Graph error!" not in call_kwargs["content"] + # Should have tried to report a failure, AND that failure must + # NOT include the raw exception text anywhere -- message, code, + # or detail (it can carry DB strings, paths, tokens, etc.). The + # full traceback only goes to the agent log via logger.exception. + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "langgraph" + assert "Graph error!" not in failure.message + assert failure.code is None + assert failure.detail is None + + @pytest.mark.asyncio + async def test_reports_error_when_graph_factory_yields_no_graph( + self, sample_message, mock_tools, mock_llm, mock_checkpointer + ): + """A bad graph factory's RuntimeError must be reported, not escape unreported.""" + adapter = LangGraphAdapter( + llm=mock_llm, + checkpointer=mock_checkpointer, + ) + await adapter.on_started("TestBot", "Test bot") + adapter.graph_factory = MagicMock(return_value=None) + + with patch( + "band.integrations.langgraph.langchain_tools.agent_tools_to_langchain" + ) as mock_convert: + mock_convert.return_value = [] + + with pytest.raises(RuntimeError, match="No graph available"): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "langgraph" diff --git a/tests/adapters/opencode/helpers.py b/tests/adapters/opencode/helpers.py index e36aef7d1..04856e97c 100644 --- a/tests/adapters/opencode/helpers.py +++ b/tests/adapters/opencode/helpers.py @@ -19,7 +19,7 @@ PlatformMessage, ) from band.integrations.opencode.types import OpencodeSessionState -from band.testing import FakeAgentTools +from band.testing import FakeAgentTools, events_of_type as events_of_type RawOpencodeEvent: TypeAlias = dict[str, Any] diff --git a/tests/adapters/opencode/test_approvals.py b/tests/adapters/opencode/test_approvals.py index 39755a829..325877a97 100644 --- a/tests/adapters/opencode/test_approvals.py +++ b/tests/adapters/opencode/test_approvals.py @@ -32,6 +32,7 @@ event_question, event_session_idle, event_text_part, + events_of_type, make_platform_message, tools_protocol, wait_for, @@ -626,8 +627,11 @@ async def test_permission_timeout_expiry() -> None: await wait_for(lambda: len(fake_client.permission_replies) > 0, timeout_s=3.0) assert fake_client.permission_replies[0]["response"] == "reject" - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] + error_events = events_of_type(tools, "error") assert any("timed out" in e["content"].lower() for e in error_events) + # A human-approval timeout is a Band-side procedural notice, never an + # AgentFailure -- it must not carry the shared failure metadata shape. + assert "failure" not in error_events[0]["metadata"] await adapter.on_cleanup("room-1") @@ -682,8 +686,11 @@ async def test_question_timeout_expiry() -> None: await wait_for(lambda: len(fake_client.question_rejections) > 0, timeout_s=3.0) assert fake_client.question_rejections == ["q-timeout"] - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] + error_events = events_of_type(tools, "error") assert any("timed out" in e["content"].lower() for e in error_events) + # A human-approval timeout is a Band-side procedural notice, never an + # AgentFailure -- it must not carry the shared failure metadata shape. + assert "failure" not in error_events[0]["metadata"] await adapter.on_cleanup("room-1") diff --git a/tests/adapters/opencode/test_lifecycle.py b/tests/adapters/opencode/test_lifecycle.py index a3b7f700a..f002f0964 100644 --- a/tests/adapters/opencode/test_lifecycle.py +++ b/tests/adapters/opencode/test_lifecycle.py @@ -25,6 +25,7 @@ event_message_updated, event_session_idle, event_text_part, + events_of_type, make_platform_message, run_single_turn, tools_protocol, @@ -259,7 +260,7 @@ async def test_concurrent_message_rejected(make_adapter, tools) -> None: ) # Second message should get rejected with "still processing" error - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] + error_events = events_of_type(tools, "error") assert any("still processing" in e["content"].lower() for e in error_events) assert len(fake_client.prompt_calls) == 1 diff --git a/tests/adapters/opencode/test_setup.py b/tests/adapters/opencode/test_setup.py index d7a457b57..3e94a9fc1 100644 --- a/tests/adapters/opencode/test_setup.py +++ b/tests/adapters/opencode/test_setup.py @@ -24,6 +24,7 @@ event_message_updated, event_session_idle, event_text_part, + events_of_type, make_platform_message, tools_protocol, ) @@ -249,7 +250,7 @@ async def test_bootstrap_creates_session_relays_text_and_persists_task( assert fake_client.created_sessions[0]["id"] == "sess-1" assert tools.messages_sent[0]["content"] == "OpenCode says hi" assert tools.messages_sent[0]["mentions"] == [{"id": "user-1"}] - task_events = [e for e in tools.events_sent if e["message_type"] == "task"] + task_events = events_of_type(tools, "task") assert task_events assert task_events[0]["metadata"]["opencode_session_id"] == "sess-1" assert ( diff --git a/tests/adapters/opencode/test_turns.py b/tests/adapters/opencode/test_turns.py index 14846a9b8..68dea6235 100644 --- a/tests/adapters/opencode/test_turns.py +++ b/tests/adapters/opencode/test_turns.py @@ -5,6 +5,8 @@ import asyncio import json +import httpx +import pytest from band.adapters.opencode import OpencodeAdapter, OpencodeAdapterConfig from band.core.types import ( @@ -12,7 +14,7 @@ Emit, ) from band.integrations.opencode.types import OpencodeSessionState -from band.testing import FakeAgentTools +from band.testing import FakeAgentTools, reported_failures from tests.adapters.usage_events import recorded_usage_payloads @@ -53,15 +55,16 @@ async def test_prompt_submission_failure_does_not_leave_room_stuck( adapter = make_adapter(fake_client) await adapter.on_started("OpenCode Agent", "A coding agent") - await adapter.on_message( - make_platform_message(content="first try"), - tools_protocol(tools), - OpencodeSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-1", - ) + with pytest.raises(httpx.HTTPStatusError): + await adapter.on_message( + make_platform_message(content="first try"), + tools_protocol(tools), + OpencodeSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) await adapter.on_message( make_platform_message(content="second try"), @@ -84,6 +87,26 @@ async def test_prompt_submission_failure_does_not_leave_room_stuck( ) +async def test_http_error_reports_status_code_as_failure_code( + make_adapter, tools +) -> None: + """An HTTP error talking to the OpenCode server preserves its status code + as the failure's ``code``, so a caller can branch on it without parsing + the message text.""" + fake_client = FakeOpencodeClient( + prompt_exceptions=[AnyHTTPStatusError(503, "sess-1")] + ) + adapter = make_adapter(fake_client) + + with pytest.raises(httpx.HTTPStatusError): + await run_single_turn(adapter, tools) + + failures = reported_failures(tools) + assert failures + assert failures[0]["provider"] == "opencode" + assert failures[0]["code"] == "503" + + async def test_reports_tool_events_when_enabled() -> None: fake_client = FakeOpencodeClient( prompt_event_sequences=[ @@ -369,9 +392,10 @@ async def test_session_error_emits_error_event(make_adapter, tools) -> None: room_id="room-1", ) - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] - assert error_events - assert "boom" in error_events[0]["content"].lower() + failures = reported_failures(tools) + assert failures + assert failures[0]["provider"] == "opencode" + assert "boom" in failures[0]["message"].lower() async def test_turn_timeout_aborts_session_and_emits_error() -> None: @@ -398,8 +422,9 @@ async def test_turn_timeout_aborts_session_and_emits_error() -> None: ) assert fake_client.aborted_sessions == ["sess-1"] - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] - assert any("timed out" in e["content"].lower() for e in error_events) + failures = reported_failures(tools) + assert any(f["provider"] == "opencode" and f["code"] == "timeout" for f in failures) + assert any("timed out" in f["message"].lower() for f in failures) await adapter.on_cleanup("room-1") @@ -714,7 +739,8 @@ async def test_task_event_post_failure_does_not_drop_the_turn(make_adapter) -> N "Handled despite the event failure." ] assert not any( - "failed while processing" in e["content"].lower() for e in tools.events_sent + "failed while processing" in f["message"].lower() + for f in reported_failures(tools) ) diff --git a/tests/adapters/test_anthropic_adapter.py b/tests/adapters/test_anthropic_adapter.py index dd8d83c3d..6011602d6 100644 --- a/tests/adapters/test_anthropic_adapter.py +++ b/tests/adapters/test_anthropic_adapter.py @@ -14,11 +14,14 @@ from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest +from anthropic import APIStatusError from anthropic.types import TextBlock, ToolUseBlock from pydantic import BaseModel, Field from band.adapters.anthropic import AnthropicAdapter +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE from band.core.types import ( USAGE_EVENT_TYPE, USAGE_METADATA_KEY, @@ -67,6 +70,7 @@ def mock_tools(): tools.get_tool_schemas = MagicMock(return_value=[]) tools.send_message = AsyncMock(return_value={"status": "sent"}) tools.send_event = AsyncMock(return_value={"status": "sent"}) + tools.send_failure = AsyncMock(return_value={"status": "sent"}) tools.execute_tool_call = AsyncMock(return_value={"status": "success"}) return tools @@ -698,6 +702,13 @@ async def test_handles_tool_error(self, mock_tools): assert "Tool failed!" in results[0]["content"] +def make_api_status_error(status_code: int, body: dict) -> APIStatusError: + """A real anthropic.APIStatusError, built the way the SDK itself would.""" + request = httpx.Request("POST", "https://api.anthropic.com/v1/messages") + response = httpx.Response(status_code, request=request, json=body) + return APIStatusError(body["error"]["message"], response=response, body=body) + + class TestErrorHandling: """Tests for error handling.""" @@ -722,7 +733,41 @@ async def test_reports_error_on_api_failure(self, sample_message, mock_tools): ) # Should have tried to report error - mock_tools.send_event.assert_called() + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "anthropic" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE + assert failure.code is None + assert failure.detail is None + + @pytest.mark.asyncio + async def test_preserves_api_status_error_as_code_and_detail( + self, sample_message, mock_tools + ): + """An APIStatusError's status_code/body are real provider data -- + preserve them rather than falling back to the generic shape.""" + adapter = AnthropicAdapter() + await adapter.on_started("TestBot", "Test bot") + body = {"error": {"type": "overloaded_error", "message": "Overloaded"}} + + with patch.object(adapter, "_call_anthropic") as mock_call: + mock_call.side_effect = make_api_status_error(529, body) + + with pytest.raises(APIStatusError): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "anthropic" + assert failure.code == "529" + assert failure.detail == body class EchoInput(BaseModel): diff --git a/tests/adapters/test_claude_sdk_adapter.py b/tests/adapters/test_claude_sdk_adapter.py index fa88d5c3d..fb40cd462 100644 --- a/tests/adapters/test_claude_sdk_adapter.py +++ b/tests/adapters/test_claude_sdk_adapter.py @@ -27,6 +27,7 @@ _FORCED_DECLINE, PendingApproval, _pre_tool_use_continue_hook, + TurnResultAlreadyReported, BAND_ALL_TOOLS, BAND_BASE_TOOLS, BAND_MEMORY_TOOLS, @@ -41,6 +42,7 @@ missing_reply_error, mcp_tool_names, ) +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE from band.core.types import Capability, Emit, PlatformMessage, ToolEventKey from claude_agent_sdk._errors import CLIConnectionError from claude_agent_sdk.types import PermissionResultAllow, ToolPermissionContext @@ -67,9 +69,8 @@ # The reply tool as the SDK namespaces it (MCP_TOOL_PREFIX + bare name). _SEND_MESSAGE_MCP_NAME = "mcp__band__band_send_message" _ANY_MODEL = "claude-sonnet-4-6" -# What a turn that ended without a reply going out must say — the "Error: " -# prefix is _report_error's own formatting, asserted by substring below -# rather than re-derived here. +# What a turn that ended without a reply going out must say, asserted by +# substring below rather than re-derived here. _MISSING_REPLY_TEXT = missing_reply_error("Claude SDK") @@ -90,12 +91,8 @@ def _tool_turn(mcp_tool_name: str) -> list: def _error_events(mock_tools: MagicMock) -> list[str]: - """Contents of the error events posted through send_event.""" - return [ - call.kwargs["content"] - for call in mock_tools.send_event.call_args_list - if call.kwargs.get("message_type") == "error" - ] + """Room-visible message of each failure reported through send_failure.""" + return [call.args[0].message for call in mock_tools.send_failure.call_args_list] def _narrated_message_types(mock_tools: MagicMock) -> list[str]: @@ -190,6 +187,7 @@ def mock_tools(): tools = MagicMock() tools.send_message = AsyncMock(return_value={"status": "sent"}) tools.send_event = AsyncMock(return_value={"status": "sent"}) + tools.send_failure = AsyncMock(return_value={"status": "sent"}) tools.add_participant = AsyncMock(return_value={"id": "user-1"}) tools.remove_participant = AsyncMock(return_value={"status": "removed"}) tools.lookup_peers = AsyncMock(return_value={"peers": []}) @@ -457,7 +455,7 @@ class TestErrorHandling: @pytest.mark.asyncio async def test_reports_error_on_query_failure(self, sample_message, mock_tools): - """When client.query raises, adapter reports error via send_event and re-raises.""" + """When client.query raises, adapter reports error via send_failure and re-raises.""" adapter = ClaudeSDKAdapter() mock_client = MagicMock() mock_client.query = AsyncMock(side_effect=Exception("API Error")) @@ -483,10 +481,10 @@ async def test_reports_error_on_query_failure(self, sample_message, mock_tools): room_id="room-123", ) - mock_tools.send_event.assert_called() - call_kwargs = mock_tools.send_event.call_args[1] - assert call_kwargs.get("message_type") == "error" - assert "API Error" in call_kwargs.get("content", "") + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "claude_sdk" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE class TestCLIConnectionError: @@ -564,10 +562,10 @@ async def test_cli_connection_error_reports_error_event( ) # Error should be surfaced to the user - mock_tools.send_event.assert_called() - call_kwargs = mock_tools.send_event.call_args[1] - assert call_kwargs.get("message_type") == "error" - assert "Process dead" in call_kwargs.get("content", "") + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "claude_sdk" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE @pytest.mark.asyncio async def test_clears_session_id_on_cli_connection_error( @@ -648,7 +646,7 @@ async def receive(): assert "room-123" not in adapter._session_ids errors = _error_events(mock_tools) assert len(errors) == 1 - assert "ended without a result" in errors[0] + assert errors[0] == GENERIC_PROVIDER_FAILURE_MESSAGE class TestRoomToolsStorage: @@ -1107,6 +1105,82 @@ async def test_falls_back_to_new_session_on_resume_failure( # Second call should be without resume second_call = mock_manager.get_or_create_session.call_args_list[1] assert second_call == (("room-123",), {"resume_session_id": None}) + # A self-healed retry is not a reportable failure. + mock_tools.send_failure.assert_not_called() + + @pytest.mark.asyncio + async def test_reports_error_when_no_stored_session_to_retry( + self, sample_message, mock_tools + ): + """No stored session id means there is nothing to fall back to, so + the failure must surface without leaking the raw exception text.""" + adapter = ClaudeSDKAdapter() + mock_manager = AsyncMock() + mock_manager.get_or_create_session = AsyncMock( + side_effect=Exception("Session setup failed") + ) + + with patch( + "band.adapters.claude_sdk.ClaudeSessionManager", + return_value=mock_manager, + ): + await adapter.on_started( + agent_name="TestBot", agent_description="A test bot" + ) + + with pytest.raises(Exception, match="Session setup failed"): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=ClaudeSDKSessionState(text=""), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "claude_sdk" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE + assert "Session setup failed" not in failure.message + + @pytest.mark.asyncio + async def test_reports_error_when_fallback_session_also_fails( + self, sample_message, mock_tools + ): + """A failure in the fallback session-creation attempt must surface + without leaking the raw exception text.""" + adapter = ClaudeSDKAdapter() + mock_manager = AsyncMock() + mock_manager.get_or_create_session = AsyncMock( + side_effect=[Exception("Resume failed"), Exception("Fresh session failed")] + ) + + with patch( + "band.adapters.claude_sdk.ClaudeSessionManager", + return_value=mock_manager, + ): + await adapter.on_started( + agent_name="TestBot", agent_description="A test bot" + ) + + with pytest.raises(Exception, match="Fresh session failed"): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=ClaudeSDKSessionState(text="", session_id="sess-broken"), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "claude_sdk" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE + assert "Fresh session failed" not in failure.message @pytest.mark.asyncio async def test_task_event_failure_does_not_break_flow(self, mock_tools): @@ -1172,11 +1246,17 @@ async def test_reports_error_on_is_error_result(self, mock_tools): ) mock_client = self._client_yielding(result_msg) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 assert "Not logged in · Please run /login" in errors[0] + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "claude_sdk" + # No structured api_error_status on this failure -- code stays unset + # rather than inventing one. + assert failure.code is None @pytest.mark.asyncio async def test_error_detail_includes_api_error_status(self, mock_tools): @@ -1186,14 +1266,19 @@ async def test_error_detail_includes_api_error_status(self, mock_tools): is_error=True, result="Failed to authenticate. API Error: 401", api_error_status=401, + errors=["authentication_error: invalid API key"], ) mock_client = self._client_yielding(result_msg) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 assert "401" in errors[0] + failure = mock_tools.send_failure.call_args.args[0] + assert failure.code == "401" + assert failure.detail == ["authentication_error: invalid API key"] @pytest.mark.asyncio async def test_reports_missing_reply_when_no_terminal_tool_ran(self, mock_tools): @@ -1202,7 +1287,8 @@ async def test_reports_missing_reply_when_no_terminal_tool_ran(self, mock_tools) result_msg = _result_message(is_error=False) mock_client = self._client_yielding(result_msg) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 @@ -1271,7 +1357,8 @@ async def test_tool_result_payload_includes_name_and_is_error(self, mock_tools): ) mock_client = self._client_yielding(assistant_msg, user_msg, _result_message()) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) payload = _tool_result_payload(mock_tools) assert payload[ToolEventKey.NAME] == "band_send_message" @@ -1285,7 +1372,8 @@ async def test_no_error_reported_when_only_read_only_tool_ran(self, mock_tools): result_msg = _result_message(is_error=False) mock_client = self._client_yielding(*turn, result_msg) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 @@ -1410,7 +1498,8 @@ async def test_declined_side_tool_still_reports_missing_reply(self, mock_tools): ) mock_client = self._client_yielding(assistant_msg, user_msg, result_msg) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 @@ -1548,7 +1637,8 @@ async def test_band_tool_error_string_is_not_terminal_work(self, mock_tools): assistant_msg, failed_result, _result_message(is_error=False) ) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 @@ -1607,7 +1697,8 @@ async def test_silent_auto_decline_still_reports_missing_reply(self, mock_tools) ) mock_client = self._client_yielding(assistant_msg, user_msg, result_msg) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 @@ -1635,7 +1726,8 @@ async def test_declined_marker_does_not_leak_into_next_turn(self, mock_tools): # A fresh, unrelated turn: no tool activity, no permission_denials. next_turn_client = self._client_yielding(_result_message(is_error=False)) - await adapter._process_response(next_turn_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(next_turn_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 @@ -1661,7 +1753,8 @@ async def test_undelivered_approval_prompt_still_reports_missing_reply( result_msg = _result_message(is_error=False) mock_client = self._client_yielding(result_msg) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 @@ -1694,7 +1787,8 @@ async def test_undelivered_timeout_notice_still_reports_missing_reply( result_msg = _result_message(is_error=False) mock_client = self._client_yielding(result_msg) - await adapter._process_response(mock_client, "room-123", mock_tools) + with pytest.raises(TurnResultAlreadyReported): + await adapter._process_response(mock_client, "room-123", mock_tools) errors = _error_events(mock_tools) assert len(errors) == 1 diff --git a/tests/adapters/test_claude_sdk_tool_names.py b/tests/adapters/test_claude_sdk_tool_names.py index 65094349d..9889aef7b 100644 --- a/tests/adapters/test_claude_sdk_tool_names.py +++ b/tests/adapters/test_claude_sdk_tool_names.py @@ -15,7 +15,7 @@ import pytest from claude_agent_sdk import AssistantMessage, ResultMessage, ToolUseBlock -from band.adapters.claude_sdk import ClaudeSDKAdapter +from band.adapters.claude_sdk import ClaudeSDKAdapter, TurnResultAlreadyReported from band.converters.claude_sdk import ClaudeSDKSessionState from band.core.types import Emit, PlatformMessage @@ -47,6 +47,7 @@ async def test_tool_call_event_uses_bare_name() -> None: ) tools = MagicMock() tools.send_event = AsyncMock(return_value={"status": "sent"}) + tools.send_failure = AsyncMock(return_value={"status": "sent"}) assistant = AssistantMessage( content=[ @@ -78,15 +79,16 @@ async def receive(): with patch("band.adapters.claude_sdk.ClaudeSessionManager", return_value=manager): await adapter.on_started(agent_name="Bot", agent_description="d") - await adapter.on_message( - msg=message, - tools=tools, - history=ClaudeSDKSessionState(text=""), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + msg=message, + tools=tools, + history=ClaudeSDKSessionState(text=""), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) tool_calls = [ call diff --git a/tests/adapters/test_codex_adapter.py b/tests/adapters/test_codex_adapter.py index b01b97dd4..d13a6941e 100644 --- a/tests/adapters/test_codex_adapter.py +++ b/tests/adapters/test_codex_adapter.py @@ -23,6 +23,10 @@ CodexAdapterConfig, PendingApproval, ) +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + TurnResultAlreadyReported, +) from band.core.types import AgentInput, Emit, HistoryProvider, PlatformMessage from band.integrations.codex import CodexJsonRpcError, RpcEvent from band.integrations.codex.types import ( @@ -30,12 +34,12 @@ CodexItemType, CodexSessionState, CodexTokenUsage, - build_structured_error_metadata, + build_agent_failure, parse_plan_steps, ) from band.runtime.custom_tools import CustomToolDef from band.runtime.tools import ToolCallOutcome -from band.testing import FakeAgentTools +from band.testing import FakeAgentTools, events_of_type, reported_failures def make_platform_message( @@ -54,11 +58,6 @@ def make_platform_message( ) -def events_of_type(tools: FakeAgentTools, message_type: str) -> list[dict[str, Any]]: - """Events of ``message_type`` captured on ``tools.events_sent``.""" - return [e for e in tools.events_sent if e["message_type"] == message_type] - - class ToolSchemaFakeTools(FakeAgentTools): def get_openai_tool_schemas(self, **kwargs: Any) -> list[dict[str, Any]]: return [ @@ -677,6 +676,24 @@ async def send_message( assert payload["decision"] == "decline" assert "room-1" not in adapter._pending_approvals + # The Band-delivery hiccup that caused this auto-decline must itself + # be reported -- otherwise it's indistinguishable from a genuine + # human decision, with no signal at all that anything went wrong. + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "codex" + + # The human sender was never actually notified, so the audit trail + # must not credit/blame them for this decision -- it was forced by + # the delivery failure, same as every other forced-decline path. + audit_events = [ + e + for e in tools.events_sent + if e["metadata"].get("codex_event_type") == "approval_resolution" + ] + assert len(audit_events) == 1 + assert audit_events[0]["metadata"]["codex_decided_by"] == "system_fallback" + @pytest.mark.asyncio async def test_cleanup_closes_client_when_last_room_removed(self) -> None: fake_client = FakeCodexClient(events=[_turn_completed()]) @@ -987,20 +1004,7 @@ async def test_models_alias_lists_models_without_starting_turn(self) -> None: @pytest.mark.asyncio async def test_reasoning_effort_passed_in_turn_overrides(self) -> None: - events = [ - _event_notification( - "turn/completed", - { - "turn": { - "id": "t1", - "threadId": "th1", - "status": "completed", - }, - "text": "Done", - }, - ), - ] - fake_client = FakeCodexClient(events=events) + fake_client = FakeCodexClient(events=[_turn_completed()]) adapter = CodexAdapter( config=CodexAdapterConfig( transport="ws", @@ -1028,20 +1032,7 @@ async def test_reasoning_effort_passed_in_turn_overrides(self) -> None: @pytest.mark.asyncio async def test_reasoning_effort_omitted_when_none(self) -> None: - events = [ - _event_notification( - "turn/completed", - { - "turn": { - "id": "t1", - "threadId": "th1", - "status": "completed", - }, - "text": "Done", - }, - ), - ] - fake_client = FakeCodexClient(events=events) + fake_client = FakeCodexClient(events=[_turn_completed()]) adapter = CodexAdapter( config=CodexAdapterConfig(transport="ws"), client_factory=lambda _config: fake_client, @@ -1109,20 +1100,7 @@ async def test_reasoning_command_rejects_invalid_effort(self) -> None: @pytest.mark.asyncio async def test_self_config_tools_registered_when_enabled(self) -> None: - events = [ - _event_notification( - "turn/completed", - { - "turn": { - "id": "t1", - "threadId": "th1", - "status": "completed", - }, - "text": "Done", - }, - ), - ] - fake_client = FakeCodexClient(events=events) + fake_client = FakeCodexClient(events=[_turn_completed()]) adapter = CodexAdapter( config=CodexAdapterConfig(transport="ws", enable_self_config_tools=True), client_factory=lambda _config: fake_client, @@ -1150,20 +1128,7 @@ async def test_self_config_tools_registered_when_enabled(self) -> None: @pytest.mark.asyncio async def test_self_config_tools_not_registered_when_disabled(self) -> None: - events = [ - _event_notification( - "turn/completed", - { - "turn": { - "id": "t1", - "threadId": "th1", - "status": "completed", - }, - "text": "Done", - }, - ), - ] - fake_client = FakeCodexClient(events=events) + fake_client = FakeCodexClient(events=[_turn_completed()]) adapter = CodexAdapter( config=CodexAdapterConfig(transport="ws", enable_self_config_tools=False), client_factory=lambda _config: fake_client, @@ -1398,20 +1363,20 @@ async def test_transport_closed_event_aborts_turn(self) -> None: tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") - await adapter.on_message( - make_platform_message(), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) - # Adapter should send a failure message mentioning the disconnect. - assert any( - "transport closed" in msg["content"].lower() for msg in tools.messages_sent - ) + # Adapter should report a failure mentioning the disconnect. + failures = reported_failures(tools) + assert any("transport closed" in f["message"].lower() for f in failures) @pytest.mark.asyncio async def test_transport_closed_resets_client_state(self) -> None: @@ -1431,15 +1396,16 @@ async def test_transport_closed_resets_client_state(self) -> None: tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") - await adapter.on_message( - make_platform_message(), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) # After transport/closed, client state should be reset assert adapter._client is None @@ -1469,15 +1435,16 @@ async def test_transport_closed_clears_per_room_state(self) -> None: adapter._room_threads["room-1"] = "old-thread-id" adapter._raw_history_by_room["room-1"] = [{"role": "user", "content": "hi"}] - await adapter.on_message( - make_platform_message(), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=False, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=False, + room_id="room-1", + ) # Per-room state should be cleared so next turn starts fresh. assert "room-1" not in adapter._room_threads @@ -1513,23 +1480,62 @@ async def test_transport_closed_drains_token_usage_for_dead_threads( input_tokens=100, total_tokens=150 ) - await adapter.on_message( - make_platform_message(), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=False, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=False, + room_id="room-1", + ) # Dead thread's usage entry must be gone even without a matching # on_cleanup (the room id can no longer look up the thread id). assert "old-thread-id" not in adapter._token_usage + @pytest.mark.asyncio + async def test_transport_closed_after_error_does_not_double_report(self) -> None: + """An "error" notification immediately followed by transport/closed for + the same incident must report the failure once, not twice -- matching + the turn/completed branch's existing failure_reported guard.""" + events = [ + _event_notification( + "error", + {"error": {"message": "Something went wrong"}, "willRetry": False}, + ), + _event_notification( + "transport/closed", + {"reason": "Codex process exited unexpectedly"}, + ), + ] + fake_client = FakeCodexClient(events=events) + adapter = CodexAdapter( + config=CodexAdapterConfig(transport="ws"), + client_factory=lambda _config: fake_client, + ) + tools = ToolSchemaFakeTools() + await adapter.on_started("Codex Agent", "A coding agent") + + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + + assert len(reported_failures(tools)) == 1 + @pytest.mark.asyncio async def test_turn_timeout_sends_interrupt_and_clean_error(self) -> None: - """When recv_event times out, the adapter sends turn/interrupt and reports cleanly.""" + """When recv_event times out, the adapter sends turn/interrupt, reports + the failure, and fails the turn so the platform retries -- same as + every sibling adapter's own turn-timeout handling.""" # No events means FakeCodexClient raises asyncio.TimeoutError immediately. fake_client = FakeCodexClient(events=[]) adapter = CodexAdapter( @@ -1539,15 +1545,16 @@ async def test_turn_timeout_sends_interrupt_and_clean_error(self) -> None: tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") - await adapter.on_message( - make_platform_message(), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) # Adapter should have sent turn/interrupt with both identifiers. interrupt_requests = [ @@ -1557,8 +1564,14 @@ async def test_turn_timeout_sends_interrupt_and_clean_error(self) -> None: ("turn/interrupt", {"threadId": "thr-1", "turnId": "turn-1"}) ] - # Adapter should send a user-facing message about stopping. - assert any("stopped" in msg["content"].lower() for msg in tools.messages_sent) + # The turn fails the platform's turn, so no separate "I stopped..." + # chat reply goes out alongside the structured failure event. + assert not tools.messages_sent + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "codex" + assert failures[0]["code"] == "timeout" @pytest.mark.asyncio async def test_item_completed_text_overrides_accumulated_deltas(self) -> None: @@ -3320,6 +3333,11 @@ async def test_explicit_model_error_propagates_without_fallback(self) -> None: model_list_calls = [m for m, _ in fake_client.requests if m == "model/list"] assert len(model_list_calls) == 0 + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "codex" + assert "not available" in failures[0]["message"] + @pytest.mark.asyncio async def test_model_selection_uses_default_when_model_list_empty(self) -> None: """Auto-selection uses the adapter default when Codex returns no visible models.""" @@ -3440,15 +3458,16 @@ async def test_codex_error_emits_event_unconditionally(self) -> None: await adapter.on_started("Agent", "An agent") msg = make_platform_message(room_id="room-1", content="do something") - await adapter.on_message( - msg, - tools, - CodexSessionState(), - None, - None, - is_session_bootstrap=False, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + msg, + tools, + CodexSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) error_events = events_of_type(tools, "error") assert len(error_events) == 1 @@ -3581,7 +3600,7 @@ async def test_structured_error_from_error_event(self) -> None: ] fake_client = FakeCodexClient(events=events) adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", structured_errors=True), + config=CodexAdapterConfig(transport="ws"), client_factory=lambda _config: fake_client, ) tools = ToolSchemaFakeTools() @@ -3597,13 +3616,12 @@ async def test_structured_error_from_error_event(self) -> None: room_id="room-1", ) - error_events = events_of_type(tools, "error") - assert len(error_events) == 1 - meta = error_events[0]["metadata"] - assert meta["codex_error_type"] == "ContextWindowExceeded" - assert meta["codex_suggested_action"] == "compact_context" - assert meta["codex_is_retryable"] is False - assert "context window" in error_events[0]["content"].lower() + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "codex" + assert failures[0]["code"] == "ContextWindowExceeded" + assert failures[0]["detail"]["codex_is_retryable"] is False + assert "context window" in failures[0]["message"].lower() @pytest.mark.asyncio async def test_structured_error_from_failed_turn(self) -> None: @@ -3629,67 +3647,93 @@ async def test_structured_error_from_failed_turn(self) -> None: ] fake_client = FakeCodexClient(events=events) adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", structured_errors=True), + config=CodexAdapterConfig(transport="ws"), client_factory=lambda _config: fake_client, ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") - await adapter.on_message( - make_platform_message(), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) - error_events = events_of_type(tools, "error") - assert len(error_events) == 1 - assert error_events[0]["metadata"]["codex_error_type"] == "UsageLimitExceeded" - assert ( - error_events[0]["metadata"]["codex_suggested_action"] == "wait_or_upgrade" - ) + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["code"] == "UsageLimitExceeded" @pytest.mark.asyncio - async def test_structured_errors_disabled_falls_back_to_plain_text(self) -> None: - """When structured_errors=False, errors use plain text format.""" + async def test_structured_error_from_failed_turn_with_no_error_key(self) -> None: + """turn/completed with status=failed but no "error" key at all must + still report a failure before raising, not just claim it did.""" events = [ _event_notification( - "error", - { - "error": { - "message": "Something failed", - "codexErrorInfo": {"type": "ContextWindowExceeded"}, - }, - "willRetry": False, - }, + "turn/completed", + {"turn": {"id": "turn-1", "status": "failed", "items": []}}, ), - _turn_completed(), ] fake_client = FakeCodexClient(events=events) adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", structured_errors=False), + config=CodexAdapterConfig(transport="ws"), client_factory=lambda _config: fake_client, ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") - await adapter.on_message( - make_platform_message(), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-1", + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "codex" + + @pytest.mark.asyncio + async def test_generic_exception_reports_and_propagates(self) -> None: + """A bare exception outside Codex's structured-error paths (not a + CodexJsonRpcError, not a delivery/already-reported failure) must + still surface via the generic fallback and propagate.""" + fake_client = FakeCodexClient( + events=[], + turn_start_error=RuntimeError("transport hiccup"), + turn_start_error_once=False, + ) + adapter = CodexAdapter( + config=CodexAdapterConfig(transport="ws"), + client_factory=lambda _config: fake_client, ) + tools = ToolSchemaFakeTools() - error_events = events_of_type(tools, "error") - assert len(error_events) == 1 - assert error_events[0]["content"] == "Codex error: Something failed" - assert "codex_error_type" not in error_events[0]["metadata"] + await adapter.on_started("Agent", "A coding agent") + with pytest.raises(RuntimeError, match="transport hiccup"): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "codex" + assert failures[0]["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE + assert "transport hiccup" not in failures[0]["message"] # =========================================================================== @@ -4649,7 +4693,7 @@ async def test_usage_command_shows_token_usage(self) -> None: class TestCodexTypes: - def test_build_structured_error_metadata_known_type(self) -> None: + def test_build_agent_failure_known_type(self) -> None: error_obj = { "message": "Context overflow", @@ -4659,25 +4703,22 @@ def test_build_structured_error_metadata_known_type(self) -> None: "retryable": False, }, } - content, meta = build_structured_error_metadata( - error_obj, thread_id="t1", turn_id="turn-1" - ) - assert "context window" in content.lower() - assert meta["codex_error_type"] == "ContextWindowExceeded" - assert meta["codex_suggested_action"] == "compact_context" - assert meta["codex_thread_id"] == "t1" - assert meta["codex_turn_id"] == "turn-1" + failure = build_agent_failure(error_obj, thread_id="t1", turn_id="turn-1") + assert failure.provider == "codex" + assert "context overflow" in failure.message.lower() + assert failure.code == "ContextWindowExceeded" + assert failure.detail["codex_thread_id"] == "t1" + assert failure.detail["codex_turn_id"] == "turn-1" - def test_build_structured_error_metadata_unknown_type(self) -> None: + def test_build_agent_failure_unknown_type(self) -> None: error_obj = { "message": "Something weird happened", "codexErrorInfo": {"type": "UnknownError"}, } - content, meta = build_structured_error_metadata(error_obj) - assert content == "Something weird happened" - assert meta["codex_error_type"] == "UnknownError" - assert meta["codex_suggested_action"] is None + failure = build_agent_failure(error_obj) + assert failure.message == "Something weird happened" + assert failure.code == "UnknownError" def test_parse_plan_steps(self) -> None: @@ -4758,9 +4799,8 @@ def test_codex_token_usage_update_current_schema(self) -> None: assert usage.total_tokens == 14822 def test_config_new_flags_default_false(self) -> None: - """All new config flags default to False (except structured_errors=True).""" + """All new config flags default to False.""" config = CodexAdapterConfig() - assert config.structured_errors is True assert config.stream_reasoning_events is False assert config.stream_plan_events is False assert config.stream_commentary_events is False @@ -5159,8 +5199,10 @@ def test_session_approval_key_empty_prevents_wildcard_match(self) -> None: assert not (key and key in {"commandExecution:npm"}) @pytest.mark.asyncio - async def test_unexpected_recv_error_still_emits_turn_outcome(self) -> None: - """When recv_event raises a non-timeout exception, _emit_turn_outcome is still called.""" + async def test_unexpected_recv_error_reports_and_fails_turn(self) -> None: + """When recv_event raises a non-timeout exception, the adapter reports + an AgentFailure and fails the turn instead of silently degrading to a + plain chat reply with no structured signal at all.""" class BrokenClient(FakeCodexClient): async def recv_event(self, timeout_s: float | None = None) -> RpcEvent: @@ -5176,17 +5218,89 @@ async def recv_event(self, timeout_s: float | None = None) -> RpcEvent: ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") - await adapter.on_message( - make_platform_message(), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-1", + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + assert len(reported_failures(tools)) == 1 + + @pytest.mark.asyncio + async def test_rpc_error_from_event_loop_keeps_curated_failure(self) -> None: + """RPC errors raised while receiving events keep their provider message.""" + rpc_error = CodexJsonRpcError(code=-32000, message="model unavailable") + + class RpcErrorClient(FakeCodexClient): + async def recv_event(self, timeout_s: float | None = None) -> RpcEvent: + raise rpc_error + + rpc_error_client = RpcErrorClient() + adapter = CodexAdapter( + config=CodexAdapterConfig(transport="ws"), + client_factory=lambda _config: rpc_error_client, ) - # Should have sent an error message to the user instead of crashing - assert any("couldn't complete" in m["content"] for m in tools.messages_sent) + tools = ToolSchemaFakeTools() + await adapter.on_started("Agent", "A coding agent") + + with pytest.raises(CodexJsonRpcError, match="model unavailable"): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["message"] == str(rpc_error) + + @pytest.mark.asyncio + async def test_failed_turn_emits_terminal_lifecycle_event(self) -> None: + """A reported failed turn still closes the lifecycle event pair.""" + events = [ + _event_notification( + "turn/completed", + {"turn": {"id": "turn-1", "status": "failed", "items": []}}, + ) + ] + fake_client = FakeCodexClient(events=events) + adapter = CodexAdapter( + config=CodexAdapterConfig( + transport="ws", + emit_turn_lifecycle_events=True, + ), + client_factory=lambda _config: fake_client, + ) + tools = ToolSchemaFakeTools() + await adapter.on_started("Agent", "A coding agent") + + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + + lifecycle_events = [ + event + for event in events_of_type(tools, "task") + if event["metadata"].get("codex_event_type") == "turn_lifecycle" + ] + assert [ + event["metadata"]["codex_turn_status"] for event in lifecycle_events + ] == ["started", "failed"] # =========================================================================== @@ -5720,7 +5834,7 @@ async def test_token_usage_event_skipped_when_total_is_zero(self) -> None: class TestStructuredErrorNormalization: - """build_structured_error_metadata handling of non-standard inputs.""" + """build_agent_failure handling of non-standard inputs.""" def test_structured_error_with_string_error_obj(self) -> None: """_handle_error_event normalizes string error_obj before structuring. @@ -5731,12 +5845,12 @@ def test_structured_error_with_string_error_obj(self) -> None: """ # Simulate the normalization the adapter performs: convert string to - # {"message": } before passing to build_structured_error_metadata. + # {"message": } before passing to build_agent_failure. error_obj: dict[str, Any] = {"message": "raw string error"} - content, meta = build_structured_error_metadata(error_obj) - assert "raw string error" in content + failure = build_agent_failure(error_obj) + assert "raw string error" in failure.message # No codexErrorInfo -> no known error type. - assert meta["codex_error_type"] is None + assert failure.code is None class TestSessionApprovalKeying: @@ -5789,6 +5903,7 @@ async def test_approve_session_refused_for_fileChange_without_paths(self) -> Non "item/fileChange/requestApproval", {"reason": "write something"}, ), + _turn_completed(), ] fake_client = FakeCodexClient(events=events) adapter = CodexAdapter( @@ -5885,58 +6000,50 @@ def test_record_approval_audit_returns_entry(self) -> None: class TestStructuredErrorMappings: - """Cover every entry in CODEX_ERROR_REMEDIATION plus the fallback path.""" + """build_agent_failure passes codexErrorInfo through verbatim -- no + remediation/suggested-action policy; that belongs to a consumer, not + this shared shape.""" @pytest.mark.parametrize( - ("error_type", "expected_action", "expected_phrase"), + "error_type", [ - ("HttpConnectionFailed", "check_connectivity", "http connection"), - ("SandboxError", "review_sandbox_policy", "sandbox"), - ("Unauthorized", "re_authenticate", "unauthorized"), - ("BadRequest", "check_input_format", "bad request"), - ( - "ResponseTooManyFailedAttempts", - "retry_different_approach", - "failed attempts", - ), + "HttpConnectionFailed", + "SandboxError", + "Unauthorized", + "BadRequest", + "ResponseTooManyFailedAttempts", ], ) - def test_known_error_type_maps_to_remediation( - self, error_type: str, expected_action: str, expected_phrase: str - ) -> None: - - content, meta = build_structured_error_metadata( + def test_error_type_becomes_the_failure_code(self, error_type: str) -> None: + failure = build_agent_failure( {"codexErrorInfo": {"type": error_type, "retryable": True}} ) - assert meta["codex_error_type"] == error_type - assert meta["codex_suggested_action"] == expected_action - assert meta["codex_is_retryable"] is True - assert expected_phrase in content.lower() + assert failure.code == error_type + assert failure.detail["codex_is_retryable"] is True def test_non_dict_codex_error_info_is_tolerated(self) -> None: - content, meta = build_structured_error_metadata( + failure = build_agent_failure( {"message": "boom", "codexErrorInfo": "not-a-dict"} ) - assert meta["codex_error_type"] is None - assert content == "boom" + assert failure.code is None + assert failure.message == "boom" def test_missing_codex_error_info_falls_back_to_message(self) -> None: - content, meta = build_structured_error_metadata({"message": "network down"}) - assert meta["codex_error_type"] is None - assert meta["codex_suggested_action"] is None - assert content == "network down" + failure = build_agent_failure({"message": "network down"}) + assert failure.code is None + assert failure.message == "network down" - def test_additional_details_preserved_in_metadata(self) -> None: + def test_additional_details_preserved_in_detail(self) -> None: - _, meta = build_structured_error_metadata( + failure = build_agent_failure( { "codexErrorInfo": {"type": "Unauthorized"}, "additionalDetails": {"hint": "refresh token"}, } ) - assert meta["codex_additional_details"] == {"hint": "refresh token"} + assert failure.detail["codex_additional_details"] == {"hint": "refresh token"} class TestSlashCommandCoverage: @@ -6050,6 +6157,92 @@ async def test_permissions_reflects_sandbox_override(self) -> None: assert len(perm_msgs) == 1 assert "read-only" in perm_msgs[0]["content"] + @pytest.mark.asyncio + async def test_local_command_reply_delivery_failure_is_not_reported( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """/help's answer failing to post is Band-side delivery, not a Codex + provider failure -- deliver_reply's DeliveryFailedError must be + recognized and left unreported here. The original cause still + propagates (the message still fails/retries at the platform level), + just never misreported as a Codex AgentFailure.""" + + class FailingSendMessageTools(ToolSchemaFakeTools): + async def send_message( + self, content: str, mentions: list[dict[str, str]] | None = None + ) -> Any: + raise RuntimeError("platform rejected the message") + + fake_client = FakeCodexClient() + adapter = CodexAdapter( + config=CodexAdapterConfig(transport="ws"), + client_factory=lambda _config: fake_client, + ) + tools = FailingSendMessageTools() + + await adapter.on_started("Agent", "A coding agent") + with ( + caplog.at_level(logging.ERROR, logger="band.core.delivery"), + pytest.raises(RuntimeError, match="platform rejected the message"), + ): + await adapter.on_message( + make_platform_message(content="/help"), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + + assert not tools.messages_sent + assert not reported_failures(tools) + assert any( + "Reply delivery failed" in record.message for record in caplog.records + ) + + @pytest.mark.asyncio + async def test_approval_command_reply_delivery_failure_is_not_reported( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """Same delivery-vs-provider-failure split as slash commands, but for + the approval-command path, which runs outside on_message's main + try/except and needs its own DeliveryFailedError handling.""" + + class FailingSendMessageTools(ToolSchemaFakeTools): + async def send_message( + self, content: str, mentions: list[dict[str, str]] | None = None + ) -> Any: + raise RuntimeError("platform rejected the message") + + fake_client = FakeCodexClient() + adapter = CodexAdapter( + config=CodexAdapterConfig(transport="ws"), + client_factory=lambda _config: fake_client, + ) + tools = FailingSendMessageTools() + + await adapter.on_started("Agent", "A coding agent") + with ( + caplog.at_level(logging.ERROR, logger="band.core.delivery"), + pytest.raises(RuntimeError, match="platform rejected the message"), + ): + await adapter.on_message( + make_platform_message(content="/approvals"), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + + assert not tools.messages_sent + assert not reported_failures(tools) + assert any( + "Reply delivery failed" in record.message for record in caplog.records + ) + class TestMalformedPayloadTolerance: """Adapter must survive notifications that are missing or misshapen.""" @@ -6063,7 +6256,7 @@ async def test_error_event_with_non_dict_error_field(self) -> None: ] fake_client = FakeCodexClient(events=events) adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", structured_errors=True), + config=CodexAdapterConfig(transport="ws"), client_factory=lambda _config: fake_client, ) tools = ToolSchemaFakeTools() @@ -6079,6 +6272,91 @@ async def test_error_event_with_non_dict_error_field(self) -> None: room_id="room-1", ) + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["message"] == "oops" + + @pytest.mark.asyncio + async def test_failed_turn_after_error_notification_reports_once(self) -> None: + """An `error` notification followed by a `turn/completed` with + status=failed for the same incident must report only one failure.""" + events = [ + _event_notification("error", {"error": {"message": "boom"}}), + _event_notification( + "turn/completed", + { + "turn": { + "id": "turn-1", + "status": "failed", + "items": [], + "error": {"message": "boom"}, + } + }, + ), + ] + fake_client = FakeCodexClient(events=events) + adapter = CodexAdapter( + config=CodexAdapterConfig(transport="ws"), + client_factory=lambda _config: fake_client, + ) + tools = ToolSchemaFakeTools() + await adapter.on_started("Agent", "A coding agent") + + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + + assert len(reported_failures(tools)) == 1 + + @pytest.mark.asyncio + async def test_failed_turn_with_falsy_scalar_error_uses_clean_fallback( + self, + ) -> None: + """A falsy, non-dict `error` (e.g. ``False``) must not become the + literal string "False" in the reported failure message.""" + events = [ + _event_notification( + "turn/completed", + { + "turn": { + "id": "turn-1", + "status": "failed", + "items": [], + "error": False, + } + }, + ), + ] + fake_client = FakeCodexClient(events=events) + adapter = CodexAdapter( + config=CodexAdapterConfig(transport="ws"), + client_factory=lambda _config: fake_client, + ) + tools = ToolSchemaFakeTools() + await adapter.on_started("Agent", "A coding agent") + + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-1", + ) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["message"] == "Codex error: unknown" + @pytest.mark.asyncio async def test_turn_completed_without_items_key(self) -> None: """turn/completed missing `items` is treated as an empty turn, not a crash.""" @@ -6256,13 +6534,13 @@ class TestStructuredErrorDetailCap: def test_long_additional_details_string_is_truncated(self) -> None: long_detail = "x" * (_MAX_ERROR_DETAIL_CHARS + 500) - _, meta = build_structured_error_metadata( + failure = build_agent_failure( { "codexErrorInfo": {"type": "Unauthorized"}, "additionalDetails": long_detail, } ) - detail = meta["codex_additional_details"] + detail = failure.detail["codex_additional_details"] assert isinstance(detail, str) assert len(detail) < len(long_detail) assert "truncated" in detail @@ -6271,24 +6549,24 @@ def test_structured_dict_additional_details_are_preserved(self) -> None: """Only string details are capped; dict/list payloads pass through.""" payload = {"hint": "refresh token", "code": 401} - _, meta = build_structured_error_metadata( + failure = build_agent_failure( { "codexErrorInfo": {"type": "Unauthorized"}, "additionalDetails": payload, } ) - assert meta["codex_additional_details"] == payload + assert failure.detail["codex_additional_details"] == payload def test_empty_additional_details_is_dropped(self) -> None: - """Empty strings are not echoed into metadata.""" + """Empty strings are not echoed into detail.""" - _, meta = build_structured_error_metadata( + failure = build_agent_failure( { "codexErrorInfo": {"type": "Unauthorized"}, "additionalDetails": "", } ) - assert "codex_additional_details" not in meta + assert failure.detail is None def test_oversized_dict_additional_details_is_replaced_with_marker( self, @@ -6305,13 +6583,13 @@ def test_oversized_dict_additional_details_is_replaced_with_marker( oversized_value = "x" * (_MAX_ERROR_DETAIL_CHARS + 500) payload = {"nested": {"blob": oversized_value}} - _, meta = build_structured_error_metadata( + failure = build_agent_failure( { "codexErrorInfo": {"type": "Unauthorized"}, "additionalDetails": payload, } ) - detail = meta["codex_additional_details"] + detail = failure.detail["codex_additional_details"] assert isinstance(detail, str) assert "truncated" in detail assert len(detail) < len(oversized_value) @@ -6325,13 +6603,13 @@ def test_unserializable_additional_details_is_dropped(self) -> None: circular: dict[str, Any] = {} circular["self"] = circular - _, meta = build_structured_error_metadata( + failure = build_agent_failure( { "codexErrorInfo": {"type": "Unauthorized"}, "additionalDetails": circular, } ) - assert "codex_additional_details" not in meta + assert failure.detail is None class TestDiffByteCap: diff --git a/tests/adapters/test_crewai_adapter.py b/tests/adapters/test_crewai_adapter.py index 48ccae12a..ef4e4232a 100644 --- a/tests/adapters/test_crewai_adapter.py +++ b/tests/adapters/test_crewai_adapter.py @@ -19,13 +19,16 @@ import json from datetime import datetime, timezone from typing import TYPE_CHECKING, Any -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import DEFAULT, AsyncMock, MagicMock import pytest from pydantic import BaseModel, Field -import band.adapters.crewai as crewai_adapter from band.adapters.crewai import EMPTY_LLM_RESPONSE_MARKER +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + TurnResultAlreadyReported, +) from band.core.types import Capability, Emit, PlatformMessage from band.runtime.prompts import render_system_prompt from band.runtime.tools import BandTool, missing_reply_error @@ -45,15 +48,6 @@ def __init__(self): pass -def error_events(mock_tools: Any) -> list[str]: - """The content of every error event the adapter posted to the room.""" - return [ - call.kwargs["content"] - for call in mock_tools.send_event.await_args_list - if call.kwargs.get("message_type") == "error" - ] - - @pytest.fixture def crewai_mocks(monkeypatch): @@ -110,6 +104,7 @@ def mock_tools(): tools.get_openai_tool_schemas = MagicMock(return_value=[]) tools.send_message = AsyncMock(return_value={"status": "sent"}) tools.send_event = AsyncMock(return_value={"status": "sent"}) + tools.send_failure = AsyncMock(return_value={"status": "sent"}) tools.execute_tool_call = AsyncMock(return_value={"status": "success"}) tools.add_participant = AsyncMock( return_value={"id": "123", "name": "Test", "status": "added"} @@ -185,6 +180,24 @@ def mock_crewai_agent(): return mock_agent +@pytest.fixture +def mock_crewai_agent_replied(mock_crewai_agent): + """``mock_crewai_agent`` whose ``kickoff_async`` also marks the turn as + replied, for tests that only care about kickoff/history/message + plumbing rather than the reply-tracking behavior itself (which has its + own dedicated tests under ``TestErrorHandling``).""" + module = importlib.import_module("band.adapters.crewai") + + def _mark_replied(*args, **kwargs): + tracker = module._reply_tracker_var.get() + if tracker is not None: + tracker.replied = True + return DEFAULT + + mock_crewai_agent.kickoff_async.side_effect = _mark_replied + return mock_crewai_agent + + @pytest.fixture def room_context(crewai_mocks, mock_tools): """Context manager fixture for setting up room context in tests. @@ -318,11 +331,11 @@ async def test_includes_platform_instructions_in_backstory( class TestOnMessage: @pytest.mark.asyncio async def test_initializes_history_on_bootstrap( - self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent + self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent_replied ): adapter = CrewAIAdapter() await adapter.on_started("TestBot", "Test bot") - adapter._crewai_agent = mock_crewai_agent + adapter._crewai_agent = mock_crewai_agent_replied await adapter.on_message( msg=sample_message, @@ -338,11 +351,11 @@ async def test_initializes_history_on_bootstrap( @pytest.mark.asyncio async def test_loads_existing_history( - self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent + self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent_replied ): adapter = CrewAIAdapter() await adapter.on_started("TestBot", "Test bot") - adapter._crewai_agent = mock_crewai_agent + adapter._crewai_agent = mock_crewai_agent_replied existing_history = [ {"role": "user", "content": "[Bob]: Previous message"}, @@ -363,11 +376,11 @@ async def test_loads_existing_history( @pytest.mark.asyncio async def test_calls_kickoff_async( - self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent + self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent_replied ): adapter = CrewAIAdapter() await adapter.on_started("TestBot", "Test bot") - adapter._crewai_agent = mock_crewai_agent + adapter._crewai_agent = mock_crewai_agent_replied await adapter.on_message( msg=sample_message, @@ -379,11 +392,11 @@ async def test_calls_kickoff_async( room_id="room-123", ) - mock_crewai_agent.kickoff_async.assert_called_once() + mock_crewai_agent_replied.kickoff_async.assert_called_once() @pytest.mark.asyncio async def test_replays_history_on_followup_turn( - self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent + self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent_replied ): """A non-bootstrap turn must replay accumulated in-session history. @@ -395,7 +408,7 @@ async def test_replays_history_on_followup_turn( """ adapter = CrewAIAdapter() await adapter.on_started("TestBot", "Test bot") - adapter._crewai_agent = mock_crewai_agent + adapter._crewai_agent = mock_crewai_agent_replied # Turn 1 (bootstrap): states something the agent must recall later. await adapter.on_message( @@ -432,7 +445,7 @@ async def test_replays_history_on_followup_turn( # The second kickoff must carry the prior turn as replayed context, not # just the current message. - prompt = mock_crewai_agent.kickoff_async.call_args_list[1][0][0] + prompt = mock_crewai_agent_replied.kickoff_async.call_args_list[1][0][0] assert "already handled" in prompt assert "Hello, agent!" in prompt # turn-1 content replayed to the model @@ -473,7 +486,10 @@ async def test_reports_error_on_kickoff_failure( room_id="room-123", ) - mock_tools.send_event.assert_called() + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "crewai" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE @pytest.mark.asyncio async def test_reports_error_when_crewai_completes_without_reply( @@ -488,21 +504,22 @@ async def test_reports_error_when_crewai_completes_without_reply( await adapter.on_started("TestBot", "Test bot") adapter._crewai_agent = mock_crewai_agent - await adapter.on_message( - msg=sample_message, - tools=mock_tools, - history=[], - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-123", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) - mock_tools.send_event.assert_awaited_once() - event_kwargs = mock_tools.send_event.await_args.kwargs - assert event_kwargs["message_type"] == "error" - assert "band_send_message" in event_kwargs["content"] - assert "max_iter=20" in event_kwargs["content"] + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "crewai" + assert "band_send_message" in failure.message + assert "max_iter=20" in failure.message @pytest.mark.asyncio async def test_reports_error_when_crewai_returns_none_without_reply( @@ -515,20 +532,21 @@ async def test_reports_error_when_crewai_returns_none_without_reply( await adapter.on_started("TestBot", "Test bot") adapter._crewai_agent = mock_crewai_agent - await adapter.on_message( - msg=sample_message, - tools=mock_tools, - history=[], - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-123", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) - mock_tools.send_event.assert_awaited_once() - event_kwargs = mock_tools.send_event.await_args.kwargs - assert event_kwargs["message_type"] == "error" - assert "band_send_message" in event_kwargs["content"] + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "crewai" + assert "band_send_message" in failure.message @pytest.mark.asyncio async def test_does_not_report_completion_error_after_reply( @@ -563,7 +581,7 @@ async def _kickoff(_messages): room_id="room-123", ) - mock_tools.send_event.assert_not_called() + mock_tools.send_failure.assert_not_awaited() @pytest.mark.asyncio async def test_suppresses_empty_final_answer_after_reply( @@ -608,8 +626,8 @@ async def _kickoff(_prompt): room_id="room-123", ) - # No error event posted to the room. - mock_tools.send_event.assert_not_called() + # No failure reported to the room. + mock_tools.send_failure.assert_not_awaited() @pytest.mark.asyncio async def test_suppresses_empty_final_answer_after_tool_only_turn( @@ -656,8 +674,8 @@ async def _kickoff(_prompt): room_id="room-123", ) - # No error event posted to the room. - mock_tools.send_event.assert_not_called() + # No failure reported to the room. + mock_tools.send_failure.assert_not_awaited() @pytest.mark.asyncio async def test_read_only_turn_with_empty_final_answer_completes( @@ -702,11 +720,13 @@ async def _kickoff(_prompt): room_id="room-123", ) - # Exactly one event, carrying the shared missing-reply wording -- the - # room hears about the missing reply, not CrewAI's internal error. - [error] = error_events(mock_tools) - assert missing_reply_error("CrewAI") in error - mock_tools.send_event.assert_awaited_once() + # Exactly one failure reported, carrying the shared missing-reply + # wording -- the room hears about the missing reply, not CrewAI's + # internal error. + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "crewai" + assert missing_reply_error("CrewAI") in failure.message @pytest.mark.asyncio async def test_empty_answer_with_no_tool_call_still_raises( @@ -741,7 +761,10 @@ async def test_empty_answer_with_no_tool_call_still_raises( room_id="room-123", ) - mock_tools.send_event.assert_awaited_once() + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "crewai" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE # First call plus the one retry -- proves the retry actually happens # before the final raise, not a bare pass-through of the first failure. assert mock_crewai_agent.kickoff_async.call_count == 2 @@ -757,13 +780,14 @@ async def test_empty_first_call_recovers_on_retry( for the retry to duplicate; the retry either behaves exactly like the first attempt would have or, as here, recovers and replies. """ + module = importlib.import_module("band.adapters.crewai") mock_result = MagicMock() mock_result.raw = "Hello! I'm here to help." async def _kickoff(_prompt): if mock_crewai_agent.kickoff_async.call_count == 1: raise ValueError(EMPTY_LLM_RESPONSE_ERROR) - tracker = crewai_adapter._reply_tracker_var.get() + tracker = module._reply_tracker_var.get() if tracker is not None: tracker.replied = True return mock_result @@ -784,7 +808,7 @@ async def _kickoff(_prompt): room_id="room-123", ) - assert error_events(mock_tools) == [] + mock_tools.send_failure.assert_not_awaited() # First call plus the one retry that recovered it. assert mock_crewai_agent.kickoff_async.call_count == 2 @@ -822,7 +846,7 @@ async def _kickoff(_prompt): room_id="room-123", ) - assert error_events(mock_tools) == [] + mock_tools.send_failure.assert_not_awaited() @pytest.mark.asyncio @pytest.mark.parametrize( @@ -871,8 +895,11 @@ async def _kickoff(_messages): room_id="room-123", ) - # And it must surface as an error event in the room. - mock_tools.send_event.assert_called() + # And it must surface as a reported failure. + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "crewai" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE @pytest.mark.asyncio async def test_raises_error_when_agent_not_initialized( @@ -893,6 +920,11 @@ async def test_raises_error_when_agent_not_initialized( room_id="room-123", ) + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "crewai" + assert "not initialized" in failure.message + class TestVerboseMode: @pytest.mark.asyncio @@ -969,11 +1001,11 @@ def test_allow_delegation_stored_on_adapter(self, CrewAIAdapter): class TestParticipantsUpdate: @pytest.mark.asyncio async def test_includes_participants_update_in_message( - self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent + self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent_replied ): adapter = CrewAIAdapter() await adapter.on_started("TestBot", "Test bot") - adapter._crewai_agent = mock_crewai_agent + adapter._crewai_agent = mock_crewai_agent_replied await adapter.on_message( msg=sample_message, @@ -985,7 +1017,7 @@ async def test_includes_participants_update_in_message( room_id="room-123", ) - call_args = mock_crewai_agent.kickoff_async.call_args + call_args = mock_crewai_agent_replied.kickoff_async.call_args prompt = call_args[0][0] assert "Alice joined" in prompt @@ -994,11 +1026,11 @@ async def test_includes_participants_update_in_message( class TestContactsUpdate: @pytest.mark.asyncio async def test_includes_contacts_update_in_message( - self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent + self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent_replied ): adapter = CrewAIAdapter() await adapter.on_started("TestBot", "Test bot") - adapter._crewai_agent = mock_crewai_agent + adapter._crewai_agent = mock_crewai_agent_replied await adapter.on_message( msg=sample_message, @@ -1010,7 +1042,7 @@ async def test_includes_contacts_update_in_message( room_id="room-123", ) - call_args = mock_crewai_agent.kickoff_async.call_args + call_args = mock_crewai_agent_replied.kickoff_async.call_args prompt = call_args[0][0] assert "@alice is now a contact" in prompt diff --git a/tests/adapters/test_crewai_adapter_soak.py b/tests/adapters/test_crewai_adapter_soak.py index 41343e06a..0a5c54cc9 100644 --- a/tests/adapters/test_crewai_adapter_soak.py +++ b/tests/adapters/test_crewai_adapter_soak.py @@ -1,13 +1,18 @@ """Soak test: 100 sequential on_message calls across 3 mocked rooms. Asserts: -- No exceptions across the run - nest_asyncio.apply is invoked at most once (lazy patch idempotency) - Per-room state in `_message_history` does not leak between rooms + +The mocked ``crewai_agent`` never drives a real ``band_send_message`` tool +call, so every turn is a genuine "missing reply" turn and raises +``TurnResultAlreadyReported`` (expected, not a soak failure) — history +bookkeeping still runs before that raise, which is what this test checks. """ from __future__ import annotations +import contextlib import importlib import sys from datetime import datetime, timezone @@ -15,6 +20,7 @@ import pytest +from band.core.protocols import TurnResultAlreadyReported from band.core.types import PlatformMessage @@ -85,15 +91,16 @@ async def test_soak_100_turns_3_rooms(crewai_mocks): for i in range(100): room_id = rooms[i % 3] msg = _make_msg(i, room_id) - await adapter.on_message( - msg=msg, - tools=tools_per_room[room_id], - history=[], - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=(i < 3), - room_id=room_id, - ) + with contextlib.suppress(TurnResultAlreadyReported): + await adapter.on_message( + msg=msg, + tools=tools_per_room[room_id], + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=(i < 3), + room_id=room_id, + ) # Per-room state present, no cross-room leakage: each room has only its own # message ids in its history. diff --git a/tests/adapters/test_crewai_flow_phase4.py b/tests/adapters/test_crewai_flow_phase4.py index 99b2f4702..543f42481 100644 --- a/tests/adapters/test_crewai_flow_phase4.py +++ b/tests/adapters/test_crewai_flow_phase4.py @@ -29,7 +29,7 @@ def _mock_crewai(monkeypatch: pytest.MonkeyPatch): RestCrewAIFlowStateSource, ) from band.core.types import PlatformMessage # noqa: E402 -from band.testing.fake_tools import FakeAgentTools # noqa: E402 +from band.testing.fake_tools import FakeAgentTools, reported_failures # noqa: E402 NS_PREFIX = "crewai_flow:" @@ -890,6 +890,47 @@ async def test_ambiguous_delegation_target_records_failed_without_sending( assert payloads[-1]["status"] == "failed" assert payloads[-1]["error"]["code"] == "ambiguous_participant" + @pytest.mark.asyncio + async def test_ambiguous_delegation_failure_message_is_capped(self) -> None: + """The ambiguous-identity error embeds every colliding participant id + -- room-visible content stays capped like every other post in this + adapter (record_waiting, reply_ambiguous), even with a large room.""" + colliding_ids = [f"participant-id-{i:040d}" for i in range(30)] + flow = _flow( + { + "decision": "delegate", + "delegations": [ + { + "delegation_id": "d-A", + "target": "peer-a", + "content": "do A", + "mentions": ["@example/peer-a"], + } + ], + } + ) + adapter = CrewAIFlowAdapter( + flow_factory=lambda: flow, + state_source=HistoryCrewAIFlowStateSource(acknowledge_test_only=True), + ) + tools = FakeAgentTools( + participants=[ + {"id": pid, "handle": "@example/peer-a", "name": "Peer A"} + for pid in colliding_ids + ] + ) + await _start(adapter, "router") + await _turn(adapter, tools, _msg(id="msg-1"), is_session_bootstrap=True) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["code"] == "ambiguous_participant" + assert len(failures[0]["message"]) <= 500 + # The full, untruncated list survives in detail for a structured + # consumer, even though the room-visible message is capped. + assert failures[0]["detail"].startswith(failures[0]["message"]) + assert len(failures[0]["detail"]) > 500 + @pytest.mark.asyncio async def test_delegation_send_failure_stops_later_delegations( self, diff --git a/tests/adapters/test_gemini_adapter.py b/tests/adapters/test_gemini_adapter.py index f6e16a189..e784bbdf1 100644 --- a/tests/adapters/test_gemini_adapter.py +++ b/tests/adapters/test_gemini_adapter.py @@ -13,6 +13,7 @@ from pydantic import BaseModel, Field, ValidationError from band.adapters.gemini import GeminiAdapter +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE from band.core.types import Emit, PlatformMessage, ToolEventKey @@ -39,6 +40,7 @@ def mock_tools() -> MagicMock: tools.get_openai_tool_schemas = MagicMock(return_value=[]) tools.send_message = AsyncMock(return_value={"status": "sent"}) tools.send_event = AsyncMock(return_value={"status": "sent"}) + tools.send_failure = AsyncMock(return_value={"status": "sent"}) tools.execute_tool_call = AsyncMock(return_value={"status": "success"}) return tools @@ -200,6 +202,93 @@ def test_extract_candidate_content_preserves_function_call_id_in_fallback(self): assert function_call.args == {"page": "1"} +class TestErrorReporting: + @pytest.mark.asyncio + async def test_reports_generic_failure(self, sample_message, mock_tools): + adapter = GeminiAdapter(provider_key="test-key") + await adapter.on_started("TestBot", "Test bot") + + with patch.object( + adapter, "_call_gemini", AsyncMock(side_effect=Exception("boom")) + ): + with pytest.raises(Exception, match="boom"): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "gemini" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE + assert failure.code is None + assert failure.detail is None + + @pytest.mark.asyncio + async def test_preserves_server_error_status_and_message( + self, sample_message, mock_tools + ): + """ServerError's status/message are real provider data -- preserve + them as code/detail rather than falling back to the generic shape.""" + adapter = GeminiAdapter(provider_key="test-key") + await adapter.on_started("TestBot", "Test bot") + error = ServerError( + 503, {"error": {"status": "UNAVAILABLE", "message": "overloaded"}}, None + ) + + with patch.object(adapter, "_call_gemini", AsyncMock(side_effect=error)): + with pytest.raises(ServerError): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "gemini" + assert failure.code == "UNAVAILABLE" + assert failure.detail == "overloaded" + + @pytest.mark.asyncio + async def test_non_string_server_error_status_is_stringified( + self, sample_message, mock_tools + ): + """A malformed error body's non-string ``status`` field must not + crash failure reporting -- band_sdk_core's AgentFailure requires + code: str | None, but ServerError.status is an unconstrained + Optional[str] at runtime (parsed straight off the response JSON).""" + adapter = GeminiAdapter(provider_key="test-key") + await adapter.on_started("TestBot", "Test bot") + error = ServerError(503, {"status": 503, "message": "backend overloaded"}, None) + + with patch.object(adapter, "_call_gemini", AsyncMock(side_effect=error)): + with pytest.raises(ServerError): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "gemini" + assert failure.code == "503" + assert failure.detail == "backend overloaded" + + class TestRetries: @pytest.mark.asyncio async def test_retries_transient_server_errors(self): @@ -582,6 +671,12 @@ async def test_raises_runtime_error_when_max_rounds_exceeded( room_id="room-123", ) + # Exceeding max tool rounds is a reportable provider failure. + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "gemini" + assert "Exceeded max tool rounds" in failure.message + class TestHttpxRetries: @pytest.mark.asyncio diff --git a/tests/adapters/test_google_adk_adapter.py b/tests/adapters/test_google_adk_adapter.py index 2746fa0ac..89fa34295 100644 --- a/tests/adapters/test_google_adk_adapter.py +++ b/tests/adapters/test_google_adk_adapter.py @@ -17,6 +17,7 @@ import pytest from pydantic import BaseModel, Field +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE from band.core.types import ALL_CAPABILITIES, Capability, Emit, PlatformMessage from band.runtime.tools import AgentTools, BandTool @@ -75,6 +76,7 @@ def mock_tools(): ) tools.send_message = AsyncMock(return_value={"status": "sent"}) tools.send_event = AsyncMock(return_value={"status": "sent"}) + tools.send_failure = AsyncMock(return_value={"status": "sent"}) tools.execute_tool_call = AsyncMock(return_value={"status": "success"}) return tools @@ -957,10 +959,40 @@ async def failing_run(**kwargs): room_id="room-123", ) - mock_tools.send_event.assert_called() + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "google_adk" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE # Runner should be closed even on error (via finally) mock_runner.close.assert_called_once() + @pytest.mark.asyncio + async def test_reports_error_when_runner_construction_itself_fails( + self, sample_message, mock_tools + ): + """A runner-construction failure must be reported, not escape uncaught.""" + adapter = GoogleADKAdapter() + await adapter.on_started("TestBot", "Test bot") + + with patch.object( + adapter, "_create_runner", side_effect=RuntimeError("bad tool schema") + ): + with pytest.raises(RuntimeError, match="bad tool schema"): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_called_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "google_adk" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE + class TestHistoryTranscript: """Tests for _format_history_transcript.""" @@ -1409,19 +1441,6 @@ def test_extract_event_text_no_text_parts(self): assert result == "" -class TestReportErrorFailure: - """Tests for _report_error own failure handling.""" - - @pytest.mark.asyncio - async def test_report_error_handles_own_failure(self, mock_tools): - """Should not raise when _report_error itself fails.""" - adapter = GoogleADKAdapter() - mock_tools.send_event = AsyncMock(side_effect=Exception("Network down")) - - # Should not raise - await adapter._report_error(mock_tools, "some error") - - class TestConcurrentMessages: """Tests for concurrent on_message calls.""" diff --git a/tests/adapters/test_letta_adapter.py b/tests/adapters/test_letta_adapter.py index bcc8cebf2..6546e2cac 100644 --- a/tests/adapters/test_letta_adapter.py +++ b/tests/adapters/test_letta_adapter.py @@ -8,6 +8,7 @@ import asyncio import json +import logging from datetime import datetime, timedelta, timezone from typing import Any from unittest.mock import AsyncMock, MagicMock @@ -21,8 +22,12 @@ RoomContext, ) from band.converters.letta import LettaSessionState +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + TurnResultAlreadyReported, +) from band.core.types import Emit -from band.testing import FakeAgentTools +from band.testing import FakeAgentTools, reported_failures from tests.adapters.lettakit import ( default_enforcement, make_assistant_message, @@ -148,6 +153,43 @@ async def test_auto_relay_when_no_send_message( assert len(tools.messages_sent) == 1 assert tools.messages_sent[0]["content"] == "I'll help you!" + @pytest.mark.asyncio + async def test_send_message_failure_is_not_reported_as_provider_failure( + self, adapter_with_client: tuple[LettaAdapter, AsyncMock] + ) -> None: + """The Letta agent answered fine; the room POST is what failed. That + must not surface as a Letta AgentFailure -- deliver_reply's + DeliveryFailedError must be recognized and left unreported here.""" + adapter, mock_client = adapter_with_client + adapter._rooms["room-1"] = RoomContext(agent_id="agent-1") + + mock_client.agents.messages.create.return_value = make_letta_response( + make_assistant_message("I'll help you!") + ) + + tools = FakeAgentTools() + + async def _raise(*args: Any, **kwargs: Any) -> None: + raise RuntimeError("platform rejected the message") + + tools.send_message = _raise # type: ignore[method-assign] + + msg = make_platform_message() + history = LettaSessionState() + + with pytest.raises(RuntimeError, match="platform rejected the message"): + await adapter.on_message( + msg, + tools, + history, + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) + + assert not reported_failures(tools) + @pytest.mark.asyncio async def test_skip_auto_relay_when_send_message_used( self, adapter_with_client: tuple[LettaAdapter, AsyncMock] @@ -198,6 +240,53 @@ async def slow_response(**kwargs: Any) -> MagicMock: msg = make_platform_message() history = LettaSessionState() + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + msg, + tools, + history, + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert "timed out" in failures[0]["message"] + assert failures[0]["provider"] == "letta" + assert failures[0]["code"] == "timeout" + + @pytest.mark.asyncio + async def test_slow_delivery_is_not_misreported_as_provider_timeout( + self, adapter_with_client: tuple[LettaAdapter, AsyncMock] + ) -> None: + """turn_timeout_s bounds only the Letta round-trip. A room POST that + is merely slow (not the Letta call) must not be misreported as a + Letta provider timeout -- it must be given time to complete.""" + adapter, mock_client = adapter_with_client + adapter.config.turn_timeout_s = 0.05 + + adapter._rooms["room-1"] = RoomContext(agent_id="agent-1") + + # The Letta call itself responds instantly, well inside turn_timeout_s. + mock_client.agents.messages.create.return_value = make_letta_response( + make_assistant_message("I'll help you!") + ) + + tools = FakeAgentTools() + real_send_message = tools.send_message + + async def _slow_send_message(*args: Any, **kwargs: Any) -> Any: + # Longer than turn_timeout_s: only the provider call may race it. + await asyncio.sleep(0.15) + return await real_send_message(*args, **kwargs) + + tools.send_message = _slow_send_message # type: ignore[method-assign] + + msg = make_platform_message() + history = LettaSessionState() + await adapter.on_message( msg, tools, @@ -208,9 +297,86 @@ async def slow_response(**kwargs: Any) -> MagicMock: room_id="room-1", ) - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] - assert len(error_events) == 1 - assert "timed out" in error_events[0]["content"] + assert len(tools.messages_sent) == 1 + assert not reported_failures(tools) + + @pytest.mark.asyncio + async def test_send_event_timeout_is_not_misreported_as_provider_timeout( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """A TimeoutError from tool-event reporting (send_event) -- not the + Letta round-trip itself -- must fall through to the generic failure + path, never be mislabeled as a Letta provider timeout. (send_failure + itself best-effort-swallows the same broken channel here, same as + production, so the room never receives a failure event either way -- + what this guards is which branch is taken/logged.)""" + config = LettaAdapterConfig() + adapter = LettaAdapter(config=config, emit=Emit.TOOL_CALLS) + mock_client = AsyncMock() + adapter._client = mock_client + adapter._system_prompt = "Test" + adapter._mcp.tool_ids = [] + adapter._mcp.server_id = "mcp-server-1" + adapter._rooms["room-1"] = RoomContext(agent_id="agent-1") + + # The Letta round-trip itself returns instantly -- well inside + # turn_timeout_s -- so any TimeoutError must come from elsewhere. + mock_client.agents.messages.create.return_value = make_letta_response( + make_tool_call_message("band_lookup_peers", "{}"), + make_tool_return_message("band_lookup_peers", '{"peers": []}'), + ) + + tools = FakeAgentTools() + tools.send_event_error = TimeoutError("event POST hiccup") + msg = make_platform_message() + history = LettaSessionState() + + with caplog.at_level(logging.ERROR, logger="band.adapters.letta"): + with pytest.raises(TimeoutError): + await adapter.on_message( + msg, + tools, + history, + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) + + assert not any("timed out" in r.message for r in caplog.records) + assert any("Error during Letta turn" in r.message for r in caplog.records) + + @pytest.mark.asyncio + async def test_generic_exception_reports_and_propagates( + self, adapter_with_client: tuple[LettaAdapter, AsyncMock] + ) -> None: + """A bare exception from the Letta client (not a timeout, not a + delivery failure) must still surface via the generic fallback.""" + adapter, mock_client = adapter_with_client + adapter._rooms["room-1"] = RoomContext(agent_id="agent-1") + mock_client.agents.messages.create.side_effect = ConnectionError( + "letta connection reset" + ) + + tools = FakeAgentTools() + msg = make_platform_message() + history = LettaSessionState() + + with pytest.raises(ConnectionError, match="letta connection reset"): + await adapter.on_message( + msg, + tools, + history, + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) + + failure = reported_failures(tools)[0] + assert failure["provider"] == "letta" + assert failure["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE + assert "letta connection reset" not in failure["message"] @pytest.mark.asyncio async def test_participants_and_contacts_injected( @@ -282,19 +448,21 @@ async def test_uninitialized_client_reports_error(self) -> None: msg = make_platform_message() history = LettaSessionState() - await adapter.on_message( - msg, - tools, - history, - None, - None, - is_session_bootstrap=True, - room_id="room-1", - ) + with pytest.raises(RuntimeError, match="not initialized"): + await adapter.on_message( + msg, + tools, + history, + None, + None, + is_session_bootstrap=True, + room_id="room-1", + ) - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] - assert len(error_events) == 1 - assert "not initialized" in error_events[0]["content"] + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "letta" + assert "not initialized" in failures[0]["message"] # ────────────────────────────────────────────────────────────────────── @@ -1232,20 +1400,22 @@ async def test_disabled_relay_fails_loud_instead_of_sending(self) -> None: ) tools = FakeAgentTools() - await adapter.on_message( - make_platform_message(), - tools, - LettaSessionState(), - None, - None, - is_session_bootstrap=False, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + tools, + LettaSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) assert len(tools.messages_sent) == 0 - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] - assert len(error_events) == 1 - assert "band_send_message" in error_events[0]["content"] + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "letta" + assert "band_send_message" in failures[0]["message"] @pytest.mark.asyncio async def test_disabled_relay_quiet_when_send_tool_used(self) -> None: @@ -1273,7 +1443,7 @@ async def test_disabled_relay_quiet_when_send_tool_used(self) -> None: ) assert len(tools.messages_sent) == 0 - assert not [e for e in tools.events_sent if e["message_type"] == "error"] + assert not reported_failures(tools) # ────────────────────────────────────────────────────────────────────── @@ -1325,6 +1495,37 @@ async def test_new_agent_in_room_with_history_is_seeded( # Seed is delivered exactly once assert adapter._rooms["room-1"].pending_seed == [] + @pytest.mark.asyncio + async def test_reported_turn_consumes_delivered_seed(self) -> None: + """A completed provider call consumes its seed before response handling.""" + adapter = LettaAdapter(config=LettaAdapterConfig(auto_relay=False)) + mock_client = AsyncMock() + adapter._client = mock_client + adapter._system_prompt = "Test prompt" + adapter._mcp.server_id = "mcp-server-1" + adapter._mcp.tool_ids = [] + room_ctx = RoomContext( + agent_id="agent-1", pending_seed=["[Alice]: Earlier context"] + ) + adapter._rooms["room-1"] = room_ctx + mock_client.agents.messages.create.return_value = make_letta_response( + make_assistant_message("The response was not sent through the tool.") + ) + + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(), + FakeAgentTools(), + LettaSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) + + assert room_ctx.pending_seed == [] + assert room_ctx.last_interaction is not None + @pytest.mark.asyncio async def test_failed_first_turn_preserves_pending_seed( self, adapter_with_client: tuple[LettaAdapter, AsyncMock] @@ -1339,15 +1540,16 @@ async def test_failed_first_turn_preserves_pending_seed( replay_messages=["[Alice]: The secret word is kumquat."] ) tools = FakeAgentTools() - await adapter.on_message( - make_platform_message(content="what was the secret word?"), - tools, - history, - None, - None, - is_session_bootstrap=True, - room_id="room-1", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + make_platform_message(content="what was the secret word?"), + tools, + history, + None, + None, + is_session_bootstrap=True, + room_id="room-1", + ) assert adapter._rooms["room-1"].pending_seed == [ "[Alice]: The secret word is kumquat." diff --git a/tests/adapters/test_letta_mcp.py b/tests/adapters/test_letta_mcp.py index 9f1f02782..aa44d9f94 100644 --- a/tests/adapters/test_letta_mcp.py +++ b/tests/adapters/test_letta_mcp.py @@ -22,12 +22,13 @@ RoomContext, ) from band.converters.letta import LettaSessionState +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE from band.integrations.letta.prompts import ( SEND_EVENT_TOOL_NAMES, SEND_MESSAGE_TOOL_NAMES, ) from band.runtime.tools import BandTool -from band.testing import FakeAgentTools +from band.testing import FakeAgentTools, reported_failures from tests.adapters.lettakit import ( make_assistant_message, make_fake_mcp_backend, @@ -519,18 +520,21 @@ async def test_failed_tool_resync_skips_turn(self) -> None: mock_client.agents.tools.list.side_effect = ConnectionError("letta hiccup") tools = FakeAgentTools() - await adapter.on_message( - make_platform_message(), - tools, - LettaSessionState(), - None, - None, - is_session_bootstrap=False, - room_id="room-1", - ) + with pytest.raises(RuntimeError, match="not attached"): + await adapter.on_message( + make_platform_message(), + tools, + LettaSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] - assert len(error_events) == 1 + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "letta" + assert failures[0]["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE mock_client.agents.messages.create.assert_not_called() @pytest.mark.asyncio @@ -765,18 +769,21 @@ async def test_prepare_failure_reports_error_and_skips_turn(self) -> None: mock_client.mcp_servers.list.side_effect = ConnectionError("letta down") tools = FakeAgentTools() - await adapter.on_message( - make_platform_message(), - tools, - LettaSessionState(), - None, - None, - is_session_bootstrap=True, - room_id="room-1", - ) + with pytest.raises(RuntimeError, match="MCP server registration failed"): + await adapter.on_message( + make_platform_message(), + tools, + LettaSessionState(), + None, + None, + is_session_bootstrap=True, + room_id="room-1", + ) - error_events = [e for e in tools.events_sent if e["message_type"] == "error"] - assert len(error_events) == 1 + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "letta" + assert failures[0]["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE mock_client.agents.messages.create.assert_not_called() @pytest.mark.asyncio diff --git a/tests/adapters/test_parlant_adapter.py b/tests/adapters/test_parlant_adapter.py index 921c4f332..aeaac635a 100644 --- a/tests/adapters/test_parlant_adapter.py +++ b/tests/adapters/test_parlant_adapter.py @@ -15,6 +15,7 @@ import pytest from band.adapters.parlant import PARLANT_PREAMBLE_TAG, ParlantAdapter +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE from band.core.types import PlatformMessage @@ -42,6 +43,7 @@ def mock_tools(): tools.get_openai_tool_schemas = MagicMock(return_value=[]) tools.send_message = AsyncMock(return_value={"status": "sent"}) tools.send_event = AsyncMock(return_value={"status": "sent"}) + tools.send_failure = AsyncMock(return_value={"status": "sent"}) tools.execute_tool_call = AsyncMock(return_value={"status": "success"}) return tools @@ -792,8 +794,110 @@ async def test_reports_error_on_failure( room_id="room-123", ) - # Should have tried to report error - mock_tools.send_event.assert_called() + # Should have tried to report the failure + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "parlant" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE + + @pytest.mark.asyncio + async def test_reports_error_on_session_init_failure( + self, mock_parlant_server, mock_parlant_agent, sample_message, mock_tools + ): + """A session-creation failure is reported, then fails the turn.""" + adapter = ParlantAdapter( + server=mock_parlant_server, + parlant_agent=mock_parlant_agent, + ) + adapter.agent_name = "TestBot" + + mock_app = MagicMock() + mock_app.sessions = AsyncMock() + mock_app.sessions.create = AsyncMock(side_effect=Exception("db unreachable")) + adapter._app = mock_app + + with pytest.raises(Exception, match="db unreachable"): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "parlant" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE + + @pytest.mark.asyncio + async def test_send_message_failure_is_not_reported_as_provider_failure( + self, mock_parlant_server, mock_parlant_agent, sample_message, mock_tools + ): + """A Band-side send_message failure while delivering the reply must + propagate as itself, not get misreported as a Parlant provider failure. + + ``deliver_reply`` wraps the ``send_message`` error in + ``DeliveryFailedError``; ``on_message``'s dedicated except branch must + re-raise the original cause before its generic ``except Exception`` + (which reports ``send_failure``) ever sees it. + """ + adapter = ParlantAdapter( + server=mock_parlant_server, + parlant_agent=mock_parlant_agent, + response_timeout=0.2, + response_poll=0.01, + ) + adapter.agent_name = "TestBot" + + agent_event = MagicMock() + agent_event.kind = "message" + agent_event.source = "ai_agent" + agent_event.offset = 2 + agent_event.data = {"message": "Hello there!", "tags": []} + + mock_app = MagicMock() + mock_app.sessions = AsyncMock() + mock_app.sessions.create = AsyncMock(return_value=MagicMock(id="session-123")) + mock_app.sessions.create_customer_message = AsyncMock( + return_value=MagicMock(offset=1) + ) + mock_app.sessions.wait_for_more_events = AsyncMock(return_value=True) + mock_app.sessions.find_events = AsyncMock(return_value=[agent_event]) + adapter._app = mock_app + + mock_tools.send_message.side_effect = ConnectionError("band down") + + mock_moderation = MagicMock() + mock_moderation.NONE = "none" + + with patch.dict( + sys.modules, + { + "parlant.core.app_modules.sessions": MagicMock( + Moderation=mock_moderation + ), + "parlant.core.sessions": MagicMock( + EventSource=MagicMock(CUSTOMER="customer", AI_AGENT="ai_agent"), + EventKind=MagicMock(MESSAGE="message"), + ), + "parlant.core.async_utils": MagicMock(Timeout=lambda x: x), + }, + ): + with pytest.raises(ConnectionError, match="band down"): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_not_awaited() @pytest.mark.asyncio async def test_clears_tools_on_error( @@ -847,26 +951,30 @@ async def test_clears_tools_on_error( async def test_handles_uninitialized_app( self, mock_parlant_server, mock_parlant_agent, sample_message, mock_tools ): - """Should handle case when app is not initialized.""" + """An uninitialized app reports the failure, then fails the turn.""" adapter = ParlantAdapter( server=mock_parlant_server, parlant_agent=mock_parlant_agent, ) # Don't set _app - # Should return early without error - await adapter.on_message( - msg=sample_message, - tools=mock_tools, - history=[], - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-123", - ) + with pytest.raises(RuntimeError, match="not initialized"): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) - # No calls should be made + # No reply attempt, but the failure is reported. mock_tools.send_message.assert_not_called() + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "parlant" + assert "not initialized" in failure.message class TestResponseWaitBudget: diff --git a/tests/adapters/test_pydantic_ai_adapter.py b/tests/adapters/test_pydantic_ai_adapter.py index 354990cdf..4fa3cdfd9 100644 --- a/tests/adapters/test_pydantic_ai_adapter.py +++ b/tests/adapters/test_pydantic_ai_adapter.py @@ -55,7 +55,11 @@ _is_output_retries_exhausted, _is_replayable_history_message, ) -from band.core.protocols import AgentToolsProtocol +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, + TurnResultAlreadyReported, +) from band.core.types import Capability, Emit, PlatformMessage, TurnUsage from band.runtime.custom_tools import get_custom_tool_name from tests.adapters.usage_events import sent_usage_payloads @@ -152,6 +156,7 @@ def mock_tools(): tools = MagicMock() tools.send_message = AsyncMock(return_value={"status": "sent"}) tools.send_event = AsyncMock(return_value={"status": "sent"}) + tools.send_failure = AsyncMock(return_value={"status": "sent"}) tools.add_participant = AsyncMock(return_value={"id": "user-1"}) tools.remove_participant = AsyncMock(return_value={"status": "removed"}) tools.lookup_peers = AsyncMock(return_value={"peers": []}) @@ -863,7 +868,10 @@ async def test_initializes_history_on_bootstrap( result_messages = [ModelRequest(parts=[UserPromptPart(content="test")])] adapter._agent.run_stream_events = MagicMock( - return_value=make_stream_events(result_messages=result_messages) + return_value=make_stream_events( + result_messages=result_messages, + tool_results=[("band_send_message", "Message sent", "call-1")], + ) ) await adapter.on_message( @@ -897,7 +905,10 @@ async def test_loads_existing_history( ModelRequest(parts=[UserPromptPart(content="new")]) ] adapter._agent.run_stream_events = MagicMock( - return_value=make_stream_events(result_messages=result_messages) + return_value=make_stream_events( + result_messages=result_messages, + tool_results=[("band_send_message", "Message sent", "call-1")], + ) ) await adapter.on_message( @@ -926,7 +937,10 @@ async def test_injects_participants_message( await adapter.on_started("TestBot", "Test bot") adapter._agent.run_stream_events = MagicMock( - return_value=make_stream_events(result_messages=[]) + return_value=make_stream_events( + result_messages=[], + tool_results=[("band_send_message", "Message sent", "call-1")], + ) ) await adapter.on_message( @@ -963,7 +977,10 @@ async def test_creates_agent_lazily_if_not_started( with patch.object(adapter, "_create_agent") as mock_create: mock_agent = MagicMock() mock_agent.run_stream_events = MagicMock( - return_value=make_stream_events(result_messages=[]) + return_value=make_stream_events( + result_messages=[], + tool_results=[("band_send_message", "Message sent", "call-1")], + ) ) mock_create.return_value = mock_agent @@ -979,6 +996,37 @@ async def test_creates_agent_lazily_if_not_started( mock_create.assert_called_once() + @pytest.mark.asyncio + async def test_reports_failure_when_no_terminal_tool_ran( + self, sample_message, mock_tools, mock_pydantic_agent + ): + """A clean run that never called a reply/terminal tool is a silently + dropped turn — must still surface as a failure, even without an + exception.""" + adapter = PydanticAIAdapter(model="openai:gpt-5.4") + with patch.object(adapter, "_create_agent", return_value=mock_pydantic_agent): + await adapter.on_started("TestBot", "Test bot") + + adapter._agent.run_stream_events = MagicMock( + return_value=make_stream_events(result_messages=[]) + ) + + with pytest.raises(TurnResultAlreadyReported): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "pydantic_ai" + assert "band_send_message" in failure.message + class TestOnCleanup: """Tests for on_cleanup() method.""" @@ -1018,7 +1066,10 @@ async def test_updates_history_after_run( ] adapter._agent.run_stream_events = MagicMock( - return_value=make_stream_events(result_messages=new_messages) + return_value=make_stream_events( + result_messages=new_messages, + tool_results=[("band_send_message", "Message sent", "call-1")], + ) ) await adapter.on_message( @@ -1072,7 +1123,10 @@ async def test_keeps_native_history_and_drops_content_null_responses( text_response, ] adapter._agent.run_stream_events = MagicMock( - return_value=make_stream_events(result_messages=result_messages) + return_value=make_stream_events( + result_messages=result_messages, + tool_results=[("band_send_message", {"id": "msg_1"}, "call_1")], + ) ) await adapter.on_message( @@ -1164,7 +1218,10 @@ async def test_ensures_history_exists_for_non_bootstrap( await adapter.on_started("TestBot", "Test bot") adapter._agent.run_stream_events = MagicMock( - return_value=make_stream_events(result_messages=[]) + return_value=make_stream_events( + result_messages=[], + tool_results=[("band_send_message", "Message sent", "call-1")], + ) ) await adapter.on_message( @@ -1201,6 +1258,7 @@ async def test_emits_tool_call_events_when_enabled( return_value=make_stream_events( result_messages=[], tool_calls=[("band_send_message", {"content": "Hello"}, "call-123")], + tool_results=[("band_send_message", "Message sent", "call-123")], ) ) @@ -1245,6 +1303,7 @@ async def test_tool_call_event_redacts_send_room_file_content( "call-123", ) ], + tool_results=[("band_send_message", "Message sent", "call-2")], ) ) @@ -1322,7 +1381,10 @@ async def test_tool_result_event_redacts_binary_content( adapter._agent.run_stream_events = MagicMock( return_value=make_stream_events( result_messages=[], - tool_results=[("band_read_room_file", [image], "call-1")], + tool_results=[ + ("band_read_room_file", [image], "call-1"), + ("band_send_message", "Message sent", "call-2"), + ], ) ) @@ -1429,9 +1491,10 @@ async def test_event_failure_does_not_crash_run( with patch.object(adapter, "_create_agent", return_value=mock_pydantic_agent): await adapter.on_started("TestBot", "Test bot") - # Mock tools where send_event fails with a real transport error (the kind - # _report_error narrowly tolerates); a generic Exception would be a bug and - # is intentionally left to propagate. + # Mock tools where send_event fails with a real transport error — the + # tool_call event's own local guard swallows this and logs a warning; + # a generic Exception would be a bug and is intentionally left to + # propagate. failing_tools = AsyncMock() failing_tools.send_event = AsyncMock( side_effect=httpx.ConnectError("Network error") @@ -1441,6 +1504,7 @@ async def test_event_failure_does_not_crash_run( return_value=make_stream_events( result_messages=[ModelRequest(parts=[UserPromptPart(content="test")])], tool_calls=[("band_send_message", {"content": "Hello"}, "call-123")], + tool_results=[("band_send_message", "Message sent", "call-123")], ) ) @@ -1554,6 +1618,7 @@ async def test_empty_output_after_tool_is_benign( isinstance(part, UserPromptPart) and "Hello, agent!" in str(part.content) for part in preserved[-1].parts ) + mock_tools.send_failure.assert_not_awaited() @pytest.mark.asyncio async def test_empty_output_preserves_full_captured_turn( @@ -1627,6 +1692,10 @@ async def test_empty_output_without_tool_propagates( room_id="room-123", ) + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "pydantic_ai" + @pytest.mark.asyncio async def test_failed_run_still_emits_captured_usage( self, sample_message, mock_tools, mock_pydantic_agent @@ -1710,6 +1779,42 @@ async def test_unrelated_model_error_propagates_even_after_tool( room_id="room-123", ) + mock_tools.send_failure.assert_awaited_once() + assert mock_tools.send_failure.call_args.args[0].provider == "pydantic_ai" + + @pytest.mark.asyncio + async def test_generic_provider_error_reports_and_propagates( + self, sample_message, mock_tools, mock_pydantic_agent + ): + """A failure that isn't UnexpectedModelBehavior at all (a raw provider/API + error) must still surface as a failure and propagate, not vanish uncaught.""" + adapter = PydanticAIAdapter(model="openai:gpt-5.4") + with patch.object(adapter, "_create_agent", return_value=mock_pydantic_agent): + await adapter.on_started("TestBot", "Test bot") + + adapter._agent.run_stream_events = MagicMock( + return_value=make_raising_stream( + RuntimeError("provider connection reset"), + tool_result=False, + ) + ) + + with pytest.raises(RuntimeError, match="provider connection reset"): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_failure.assert_awaited_once() + failure = mock_tools.send_failure.call_args.args[0] + assert failure.provider == "pydantic_ai" + assert failure.message == GENERIC_PROVIDER_FAILURE_MESSAGE + @pytest.mark.asyncio async def test_empty_output_after_read_only_tool_propagates( self, sample_message, mock_tools, mock_pydantic_agent @@ -1894,7 +1999,10 @@ async def my_helper(ctx: RunContext[AgentToolsProtocol], value: str) -> str: result_messages = [ModelRequest(parts=[UserPromptPart(content="test")])] adapter._agent.run_stream_events = MagicMock( - return_value=make_stream_events(result_messages=result_messages) + return_value=make_stream_events( + result_messages=result_messages, + tool_results=[("band_send_message", "Message sent", "call-1")], + ) ) # Should not raise diff --git a/tests/adapters/test_strands_adapter.py b/tests/adapters/test_strands_adapter.py index 975750efe..130563408 100644 --- a/tests/adapters/test_strands_adapter.py +++ b/tests/adapters/test_strands_adapter.py @@ -35,7 +35,11 @@ _tool_result, ) from band.converters.strands import StrandsHistoryConverter # noqa: E402 -from band.core.protocols import AgentToolsProtocol # noqa: E402 +from band.core.protocols import ( # noqa: E402 + GENERIC_PROVIDER_FAILURE_MESSAGE, + AgentToolsProtocol, + TurnResultAlreadyReported, +) from band.core.types import ( # noqa: E402 USAGE_METADATA_KEY, AgentInput, @@ -53,6 +57,7 @@ ScriptedStrandsModel, ScriptedTurn, ToolTurn, + reported_failures, ) _INPUT_TOKENS_PER_CALL = 7 @@ -142,10 +147,6 @@ def _alternates(history: list) -> bool: return all(first != second for first, second in zip(roles, roles[1:])) -def _errors(tools: FakeAgentTools) -> list[str]: - return [e["content"] for e in tools.events_sent if e["message_type"] == "error"] - - class TestCustomToolWiring: def test_custom_tool_def_converted_to_bridge(self): class WeatherInput(BaseModel): @@ -447,11 +448,16 @@ async def test_later_turns_keep_the_transcript_the_adapter_owns( A later turn that reseeded would replay the room's own transcript on top of the one the adapter is already holding. """ - adapter = await scripted(SEND_TURN, SEND_TURN) + adapter = await scripted(SEND_TURN) await _run_message(adapter, tools, history=[]) after_first = list(adapter._message_history[ROOM]) - await _run_message(adapter, tools, history=[], is_session_bootstrap=False) + # The scripted model has no turn left for a second reply, so this + # turn ends without calling band_send_message -- irrelevant to what + # this test checks (the transcript isn't re-seeded), so only the + # failure is asserted here, not suppressed. + with pytest.raises(TurnResultAlreadyReported): + await _run_message(adapter, tools, history=[], is_session_bootstrap=False) assert adapter._message_history[ROOM][: len(after_first)] == after_first @@ -526,10 +532,13 @@ async def send_message(self, content, mentions=None): tools = FailingTools(room_id=ROOM) adapter = await scripted(SEND_TURN) - await _run_message(adapter, tools) + with pytest.raises(TurnResultAlreadyReported): + await _run_message(adapter, tools) assert tools.messages_sent == [] - assert len(_errors(tools)) == 1 + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "strands" # The shared bridge returns a normalized, model-visible tool failure. assert any( text.startswith("Error executing band_send_message:") @@ -548,7 +557,8 @@ async def send_message(self, content, mentions=None): tools = FailingTools(room_id=ROOM) adapter = await scripted(SEND_TURN, emit=Emit.TOOL_CALLS) - await _run_message(adapter, tools) + with pytest.raises(TurnResultAlreadyReported): + await _run_message(adapter, tools) rehydrated = StrandsHistoryConverter(agent_name="Bot").convert( [ @@ -570,11 +580,14 @@ async def test_read_only_tool_alone_does_not_end_the_turn(self, tools, scripted) """Looking peers up succeeds but posts nothing, so the reply is still missing.""" adapter = await scripted(ToolTurn("band_lookup_peers", {})) - await _run_message(adapter, tools) + with pytest.raises(TurnResultAlreadyReported): + await _run_message(adapter, tools) assert _tool_results(adapter) # the lookup did run and succeed assert tools.messages_sent == [] - assert "band_send_message" in _errors(tools)[0] + failure = reported_failures(tools)[0] + assert failure["provider"] == "strands" + assert "band_send_message" in failure["message"] @pytest.mark.asyncio async def test_invalid_tool_arguments_are_answered_not_raised( @@ -583,7 +596,8 @@ async def test_invalid_tool_arguments_are_answered_not_raised( """A malformed call is the model's mistake to correct, not a turn-ending crash.""" adapter = await scripted(ToolTurn("band_send_message", {"mentions": ["@x"]})) - await _run_message(adapter, tools) + with pytest.raises(TurnResultAlreadyReported): + await _run_message(adapter, tools) assert tools.messages_sent == [] assert _tool_results(adapter) == [ @@ -604,7 +618,8 @@ async def boom(args: BoomInput) -> str: ToolTurn("boom", {"note": "go"}), additional_tools=[(BoomInput, boom)] ) - await _run_message(adapter, tools) + with pytest.raises(TurnResultAlreadyReported): + await _run_message(adapter, tools) assert _tool_results(adapter) == ["Error executing tool 'boom': no network"] @@ -635,6 +650,9 @@ async def test_provider_failure_keeps_the_transcript_and_reports_usage( assert usage[0]["metadata"][USAGE_METADATA_KEY]["input_tokens"] == ( _INPUT_TOKENS_PER_CALL ) + failure = reported_failures(tools)[0] + assert failure["provider"] == "strands" + assert failure["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE class TestUsageMapping: @@ -779,7 +797,8 @@ async def test_tool_call_event_redacts_content_not_raw_bytes(self, tools): ) await adapter.on_started("Bot", "A bot") - await _run_message(adapter, tools) + with pytest.raises(TurnResultAlreadyReported): + await _run_message(adapter, tools) tool_calls = [ json.loads(e["content"]) diff --git a/tests/conftest.py b/tests/conftest.py index 5c9f68553..f6c6877a9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,8 +15,13 @@ from __future__ import annotations -import asyncio import os + +# Must be set before crewai is first imported: its event bus installs a +# global OpenTelemetry provider and a live exporter thread at import time. +os.environ.setdefault("CREWAI_DISABLE_TELEMETRY", "true") + +import asyncio from datetime import datetime, timezone from functools import cache from itertools import count diff --git a/tests/core/test_delivery.py b/tests/core/test_delivery.py new file mode 100644 index 000000000..f863513ff --- /dev/null +++ b/tests/core/test_delivery.py @@ -0,0 +1,32 @@ +"""Tests for the delivery-vs-provider-failure misclassification guard.""" + +from __future__ import annotations + +import pytest + +from band.core.delivery import DeliveryFailedError, deliver_reply +from band.testing.fake_tools import FakeAgentTools + + +class TestDeliverReply: + async def test_forwards_a_successful_send(self) -> None: + tools = FakeAgentTools() + + result = await deliver_reply(tools, "hello", mentions=["@alice"]) + + assert tools.messages_sent[0]["content"] == "hello" + assert result["content"] == "hello" + + async def test_wraps_a_send_message_failure(self) -> None: + """A raised send_message must become DeliveryFailedError, not + propagate as-is -- so a shared except block reading for a provider + failure can tell delivery and provider failures apart.""" + tools = FakeAgentTools() + + with pytest.raises(DeliveryFailedError) as exc_info: + # FakeAgentTools.send_message raises BandToolError for a + # mention-less send, mirroring the real platform requirement. + await deliver_reply(tools, "hello", mentions=None) + + assert exc_info.value.__cause__ is exc_info.value.cause + assert "mention" in str(exc_info.value.cause).lower() diff --git a/tests/core/test_protocols.py b/tests/core/test_protocols.py new file mode 100644 index 000000000..0e4bf2828 --- /dev/null +++ b/tests/core/test_protocols.py @@ -0,0 +1,101 @@ +"""Tests for shared AgentToolsProtocol helpers.""" + +from __future__ import annotations + +import logging + +import pytest +from band_sdk_core import AgentFailure + +from band.core.protocols import send_event_safe, to_failure_event +from band.testing.fake_tools import FakeAgentTools + + +class TestToFailureEvent: + """``to_failure_event`` is the one place both SDKs put the failure -> + room-event shape, so its parity string and metadata key are load-bearing.""" + + def test_carries_the_message_and_failure_metadata(self) -> None: + failure = AgentFailure("codex", "boom", "timeout", {"http_status": 500}) + + content, metadata = to_failure_event(failure) + + assert content == "boom" + assert metadata == { + "failure": { + "provider": "codex", + "code": "timeout", + "message": "boom", + "detail": {"http_status": 500}, + } + } + + @pytest.mark.parametrize("blank_message", ["", " ", "\n\t"]) + def test_blank_message_falls_back_to_a_generic_one( + self, blank_message: str + ) -> None: + """A provider message can arrive blank -- the platform rejects a blank + chat event, so an unguarded blank message would make the failure + vanish from the room entirely. The fallback string must match TS's + ``toFailureEvent`` exactly for cross-SDK parity.""" + content, _metadata = to_failure_event(AgentFailure("acp", blank_message)) + + assert content == "acp failed without an error message." + + def test_generic_fallback_has_no_code_or_detail(self) -> None: + """A provider/adapter that gives no structured signal at all still + produces a valid failure -- code and detail default to None rather + than an invented value.""" + content, metadata = to_failure_event(AgentFailure("anthropic", "boom")) + + assert content == "boom" + assert metadata["failure"]["code"] is None + assert metadata["failure"]["detail"] is None + + +class TestSendEventSafe: + """send_event_safe is the shared best-effort event sender every migrated + adapter's non-critical telemetry (thoughts, task/lifecycle markers) goes + through -- a regression here silently drops events across every one of + them.""" + + async def test_forwards_a_successful_send_and_returns_true(self) -> None: + tools = FakeAgentTools() + + sent = await send_event_safe(tools, "hello", "thought") + + assert sent is True + assert tools.events_sent[0]["content"] == "hello" + + async def test_swallows_a_send_event_failure_and_logs_at_the_given_level( + self, caplog: pytest.LogCaptureFixture + ) -> None: + tools = FakeAgentTools() + tools.send_event_error = RuntimeError("platform rejected the event") + + with caplog.at_level(logging.DEBUG, logger="band.core.protocols"): + sent = await send_event_safe( + tools, + "hello", + "task", + log_label="widget event", + log_level=logging.DEBUG, + ) + + assert sent is False + assert tools.events_sent == [] + record = next(r for r in caplog.records if r.name == "band.core.protocols") + assert record.levelno == logging.DEBUG + assert "widget event" in record.message + + async def test_default_log_level_is_warning( + self, caplog: pytest.LogCaptureFixture + ) -> None: + tools = FakeAgentTools() + tools.send_event_error = RuntimeError("boom") + + with caplog.at_level(logging.WARNING, logger="band.core.protocols"): + await send_event_safe(tools, "hello", "task") + + record = next(r for r in caplog.records if r.name == "band.core.protocols") + assert record.levelno == logging.WARNING diff --git a/tests/docker/launcher/conftest.py b/tests/docker/launcher/conftest.py index 5090b83d6..7350acee0 100644 --- a/tests/docker/launcher/conftest.py +++ b/tests/docker/launcher/conftest.py @@ -7,6 +7,7 @@ from __future__ import annotations +import os from pathlib import Path import pytest @@ -30,6 +31,13 @@ def as_agent_uid(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(launcher_run, "current_uid", lambda: AGENT_UID) +@pytest.fixture(autouse=True) +def restore_home(monkeypatch: pytest.MonkeyPatch) -> None: + """main() sets the real process HOME as a deliberate side effect; pin it + through monkeypatch so a test calling main() in-process doesn't leak it.""" + monkeypatch.setenv("HOME", os.environ.get("HOME", "")) + + @pytest.fixture def workspace(tmp_path: Path) -> Workspace: return make_workspace(tmp_path) diff --git a/tests/framework_configs/adapters.py b/tests/framework_configs/adapters.py index 952468310..add0c3f7c 100644 --- a/tests/framework_configs/adapters.py +++ b/tests/framework_configs/adapters.py @@ -625,10 +625,10 @@ def _build_codex_config() -> AdapterConfig: "config": CodexAdapterConfig(), }, custom_kwargs={ - "config": CodexAdapterConfig(structured_errors=False), + "config": CodexAdapterConfig(stream_plan_events=True), }, custom_expected={ - "config": CodexAdapterConfig(structured_errors=False), + "config": CodexAdapterConfig(stream_plan_events=True), }, has_custom_tools_attr=True, custom_tools_attr="_custom_tools", diff --git a/tests/framework_conformance/test_strands_injection_spike.py b/tests/framework_conformance/test_strands_injection_spike.py index 9ef8ed54a..69875fabc 100644 --- a/tests/framework_conformance/test_strands_injection_spike.py +++ b/tests/framework_conformance/test_strands_injection_spike.py @@ -66,13 +66,17 @@ pytest.importorskip("strands", reason="strands extra not installed") from band.adapters.strands import StrandsAdapter # noqa: E402 -from band.core.protocols import AgentToolsProtocol # noqa: E402 +from band.core.protocols import ( # noqa: E402 + AgentToolsProtocol, + TurnResultAlreadyReported, +) from band.core.types import Emit, PlatformMessage # noqa: E402 from band.testing import ( # noqa: E402 FakeAgentTools, ScriptedStrandsModel, TextTurn, ToolTurn, + reported_failures, ) _SEND_CONTENT = "Injected reply: PINEAPPLE" @@ -205,13 +209,17 @@ async def test_negative_control_text_only_sends_no_message() -> None: adapter = StrandsAdapter( model=ScriptedStrandsModel([TextTurn("just a reply, no tools")]) ) - await _run(adapter, tools, room_id) + with pytest.raises(TurnResultAlreadyReported): + await _run(adapter, tools, room_id) assert tools.messages_sent == [], ( f"expected no send for a text-only decision, got: {tools.messages_sent}" ) assert tools.tool_calls == [] # The plain-text answer was silently dropped — the adapter must surface it. - errors = [e for e in tools.events_sent if e["message_type"] == "error"] - assert len(errors) == 1, f"expected one error event, got: {tools.events_sent}" - assert "band_send_message" in errors[0]["content"] + failures = reported_failures(tools) + assert len(failures) == 1, ( + f"expected one reported failure, got: {tools.events_sent}" + ) + assert failures[0]["provider"] == "strands" + assert "band_send_message" in failures[0]["message"] diff --git a/tests/integrations/a2a/gateway/test_adapter.py b/tests/integrations/a2a/gateway/test_adapter.py index f7be7d9cf..0397ddf6f 100644 --- a/tests/integrations/a2a/gateway/test_adapter.py +++ b/tests/integrations/a2a/gateway/test_adapter.py @@ -4,6 +4,7 @@ import asyncio from datetime import datetime +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch from uuid import uuid4 @@ -23,7 +24,11 @@ from band.core.types import PlatformMessage from band.client.rest import DEFAULT_REQUEST_OPTIONS from band.integrations.a2a.gateway import A2AGatewayAdapter, A2AGatewayAdapterConfig -from band.integrations.a2a.gateway.adapter import BandAgentExecutor, GatewayRequest +from band.integrations.a2a.gateway.adapter import ( + BandAgentExecutor, + GatewayRequest, + _redact_credentials, +) from band.integrations.a2a.gateway.types import GatewaySessionState, PendingA2ATask from band.testing import FakeAgentTools from tests.integrations.a2a.gateway.helpers import make_peer, peers_page @@ -33,6 +38,7 @@ def make_platform_message( content: str, room_id: str = "room-123", message_type: str = "text", + metadata: dict[str, Any] | None = None, ) -> PlatformMessage: return PlatformMessage( id=str(uuid4()), @@ -42,7 +48,7 @@ def make_platform_message( sender_type="Agent", sender_name="Weather Agent", message_type=message_type, - metadata={}, + metadata=metadata if metadata is not None else {}, created_at=datetime.now(), ) @@ -295,6 +301,8 @@ async def test_timeout_returns_terminal_failure( await queue.dequeue_event() terminal = await queue.dequeue_event() assert terminal.status.state == TaskState.TASK_STATE_FAILED + assert terminal.metadata["failure"]["provider"] == "a2a-gateway" + assert terminal.metadata["failure"]["code"] == "timeout" assert adapter._pending_tasks == {} assert not any( "A2A request completed" in record.message for record in caplog.records @@ -322,6 +330,58 @@ async def test_send_failure_publishes_terminal_failure(self) -> None: assert terminal.status.message.parts[0].text == "A2A request failed" assert "Band unavailable" not in terminal.status.message.parts[0].text assert adapter._pending_tasks == {} + failure = terminal.metadata["failure"] + assert failure["provider"] == "a2a-gateway" + assert failure["code"] == "RuntimeError" + assert "Band unavailable" in failure["message"] + + @pytest.mark.asyncio + async def test_send_failure_redacts_secrets_from_reported_metadata(self) -> None: + """The sanitized exception text reaches the A2A client's metadata -- + a leaked bearer token or API key must not.""" + adapter = A2AGatewayAdapter(rest_client=MagicMock()) + adapter._peers = {"weather": make_peer("weather", "Weather Agent")} + configure_room_creation(adapter) + adapter._rest.agent_api_messages.create_agent_chat_message = AsyncMock( + side_effect=RuntimeError( + "upstream rejected Bearer abc123.def456 (api_key=sk-live-secret)" + ) + ) + queue = EventQueueLegacy() + + with pytest.raises(RuntimeError): + await BandAgentExecutor(adapter, "weather").execute(make_request(), queue) + + await queue.dequeue_event() + terminal = await queue.dequeue_event() + message = terminal.metadata["failure"]["message"] + assert "abc123.def456" not in message + assert "sk-live-secret" not in message + assert "Bearer [REDACTED]" in message + assert "api_key=[REDACTED]" in message + + def test_redact_credentials_full_value_scheme_prefixed(self) -> None: + """A scheme-prefixed credential value (a space between the key and + the secret) must be redacted in full, not just up to that space.""" + redacted = _redact_credentials("Authorization: ApiKey sk-live-abcdef123456") + assert "sk-live-abcdef123456" not in redacted + assert redacted == "Authorization=[REDACTED]" + + @pytest.mark.parametrize( + "text", + [ + "password=hunter2", + "client_secret=abc123XYZ", + "AWS_SECRET_ACCESS_KEY=AKIAABCDEFGHIJKLMNOP", + ], + ) + def test_redact_credentials_covers_non_token_keywords(self, text: str) -> None: + """token/authorization/api_key aren't the only credential-shaped + keywords a peer's error text can embed -- password, secret (and its + client_secret compound), and access_key must be redacted too.""" + redacted = _redact_credentials(text) + secret_value = text.split("=", 1)[1] + assert secret_value not in redacted @pytest.mark.asyncio async def test_establish_request_raises_when_peer_missing(self) -> None: @@ -378,6 +438,7 @@ async def test_cleanup_all_fails_inflight_requests(self) -> None: terminal = await queue.dequeue_event() assert terminal.status.state == TaskState.TASK_STATE_FAILED + assert not terminal.metadata, "a gateway shutdown is not a provider failure" assert pending.done.is_set() assert adapter._pending_tasks == {} @@ -409,6 +470,7 @@ async def test_room_cleanup_returns_terminal_failure(self) -> None: terminal = await queue.dequeue_event() assert terminal.status.state == TaskState.TASK_STATE_FAILED + assert not terminal.metadata, "a room closing is not a provider failure" assert pending.done.is_set() assert adapter._pending_tasks == {} @@ -547,3 +609,139 @@ async def test_publishes_band_message_with_matching_task_state( assert event.status.state == state assert event.status.message.parts[0].text == "response" + + @pytest.mark.asyncio + async def test_relays_peers_own_agent_failure_unchanged(self) -> None: + """The peer's adapter already built this AgentFailure (send_failure) -- + the gateway must relay it as-is, not re-tag its provider as + "a2a-gateway".""" + adapter = A2AGatewayAdapter(rest_client=MagicMock()) + queue = EventQueueLegacy() + pending = make_pending(queue) + peer_failure = { + "provider": "codex", + "code": "ContextWindowExceeded", + "message": "context window exceeded", + "detail": None, + } + + await adapter._publish_band_response( + pending, + make_platform_message( + "context window exceeded", + message_type="error", + metadata={"failure": peer_failure}, + ), + ) + event = await queue.dequeue_event() + + assert event.status.state == TaskState.TASK_STATE_FAILED + assert event.metadata["failure"]["provider"] == "codex" + assert event.metadata["failure"]["code"] == "ContextWindowExceeded" + + @pytest.mark.asyncio + async def test_relayed_peer_failure_redacts_embedded_credentials(self) -> None: + """A peer's own AgentFailure can embed a raw provider exception + message -- redact it the same as this gateway's own exception path + before it reaches an external A2A client.""" + adapter = A2AGatewayAdapter(rest_client=MagicMock()) + queue = EventQueueLegacy() + pending = make_pending(queue) + secret_message = "upstream rejected token=sk-live-secret" + peer_failure = { + "provider": "codex", + "code": "Unauthorized", + "message": secret_message, + "detail": None, + } + + await adapter._publish_band_response( + pending, + make_platform_message( + secret_message, + message_type="error", + metadata={"failure": peer_failure}, + ), + ) + event = await queue.dequeue_event() + + assert event.status.state == TaskState.TASK_STATE_FAILED + assert "sk-live-secret" not in event.metadata["failure"]["message"] + assert "sk-live-secret" not in event.status.message.parts[0].text + + @pytest.mark.asyncio + async def test_relayed_peer_failure_redacts_nested_credentials_in_detail( + self, + ) -> None: + """A peer's AgentFailure.detail can nest a credential-bearing string + inside a dict/list (e.g. Codex's own codex_additional_details) -- + _redact_credentials_deep must recurse into it, not just the flat + message string.""" + adapter = A2AGatewayAdapter(rest_client=MagicMock()) + queue = EventQueueLegacy() + pending = make_pending(queue) + peer_failure = { + "provider": "codex", + "code": "Unauthorized", + "message": "upstream rejected the request", + "detail": { + "codex_additional_details": { + "raw": ["upstream said: token=sk-live-nested-secret"], + }, + }, + } + + await adapter._publish_band_response( + pending, + make_platform_message( + "upstream rejected the request", + message_type="error", + metadata={"failure": peer_failure}, + ), + ) + event = await queue.dequeue_event() + + assert event.status.state == TaskState.TASK_STATE_FAILED + detail = event.metadata["failure"]["detail"] + assert "sk-live-nested-secret" not in str(detail) + + @pytest.mark.asyncio + async def test_drops_non_dict_peer_failure_metadata(self) -> None: + """Malformed peer failure metadata must not cross the A2A boundary.""" + adapter = A2AGatewayAdapter(rest_client=MagicMock()) + queue = EventQueueLegacy() + pending = make_pending(queue) + secret = "password=peer-secret" + + await adapter._publish_band_response( + pending, + make_platform_message( + secret, + message_type="error", + metadata={"failure": secret}, + ), + ) + event = await queue.dequeue_event() + + assert event.status.state == TaskState.TASK_STATE_FAILED + assert "failure" not in event.metadata + assert secret not in event.status.message.parts[0].text + + @pytest.mark.asyncio + async def test_plain_error_message_without_failure_metadata_still_fails( + self, + ) -> None: + """A peer that never migrated to send_failure still fails the task -- + it just carries no structured metadata.""" + adapter = A2AGatewayAdapter(rest_client=MagicMock()) + queue = EventQueueLegacy() + pending = make_pending(queue) + + await adapter._publish_band_response( + pending, + make_platform_message("something broke", message_type="error"), + ) + event = await queue.dequeue_event() + + assert event.status.state == TaskState.TASK_STATE_FAILED + assert not event.metadata diff --git a/tests/integrations/a2a/test_adapter.py b/tests/integrations/a2a/test_adapter.py index 1d7ed508e..b5aa24dde 100644 --- a/tests/integrations/a2a/test_adapter.py +++ b/tests/integrations/a2a/test_adapter.py @@ -22,10 +22,15 @@ TaskStatus, ) +from band.core.delivery import DeliveryFailedError +from band.core.protocols import ( + GENERIC_PROVIDER_FAILURE_MESSAGE, + TurnResultAlreadyReported, +) from band.core.types import PlatformMessage from band.integrations.a2a import A2AAdapter, A2AAuth, A2ASessionState from band.integrations.a2a.adapter import _SSE_READ_TIMEOUT_S -from band.testing import FakeAgentTools +from band.testing import FakeAgentTools, reported_failures def make_platform_message(content: str = "Hello") -> PlatformMessage: @@ -298,10 +303,11 @@ async def test_terminal_task_is_finalized_even_when_band_delivery_fails( tools.send_message = AsyncMock(side_effect=RuntimeError("Band unavailable")) task = make_task(artifact_text="Final response") - with pytest.raises(RuntimeError, match="Band unavailable"): + with pytest.raises(DeliveryFailedError) as exc_info: await adapter._handle_event( task_event(task), tools, "room-123", "user-456", "Test User" ) + assert "Band unavailable" in str(exc_info.value.cause) assert tools.events_sent[-1]["metadata"]["a2a_task_state"] == ( "TASK_STATE_COMPLETED" @@ -311,30 +317,53 @@ async def test_terminal_task_is_finalized_even_when_band_delivery_fails( assert adapter._task_senders == {} @pytest.mark.asyncio - async def test_auth_required_task_is_posted_as_error_event( + async def test_finally_block_failure_does_not_replace_try_blocks_exception( self, adapter: A2AAdapter ) -> None: + """The terminal task-event emission in ``finally`` must never clobber + a ``DeliveryFailedError`` already propagating from the try block -- + Python's try/finally semantics otherwise let the finally's own + exception silently replace it.""" tools = FakeAgentTools() + tools.send_message = AsyncMock(side_effect=RuntimeError("Band unavailable")) + tools.send_event_error = RuntimeError("task event post also failed") + task = make_task(artifact_text="Final response") + + with pytest.raises(DeliveryFailedError) as exc_info: + await adapter._handle_event( + task_event(task), tools, "room-123", "user-456", "Test User" + ) + assert "Band unavailable" in str(exc_info.value.cause) + assert adapter._tasks == {}, "next turn must start a fresh task" + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "state", + [TaskState.TASK_STATE_CANCELED, TaskState.TASK_STATE_AUTH_REQUIRED], + ) + async def test_non_retryable_terminal_task_is_acked_after_error_event( + self, adapter: A2AAdapter, state: int + ) -> None: + tools = FakeAgentTools() + status_message = ( + "Please authenticate" + if state == TaskState.TASK_STATE_AUTH_REQUIRED + else "The task was canceled" + ) await adapter._handle_event( - task_event( - make_task( - TaskState.TASK_STATE_AUTH_REQUIRED, - status_message="Please authenticate", - ) - ), + task_event(make_task(state, status_message=status_message)), tools, "room-123", "user-456", "Test User", ) - error_events = [ - event for event in tools.events_sent if event["message_type"] == "error" - ] - assert error_events, "an auth-required task must produce an error event" - assert error_events[-1]["content"] == "Please authenticate" - assert error_events[-1]["metadata"]["a2a_state"] == "TASK_STATE_AUTH_REQUIRED" + failures = reported_failures(tools) + assert failures, "a non-retryable terminal task must produce an error event" + assert failures[-1]["message"] == status_message + assert failures[-1]["provider"] == "a2a" + assert failures[-1]["code"] == TaskState.Name(state) @pytest.mark.asyncio async def test_input_required_is_forwarded_and_persisted( @@ -364,25 +393,55 @@ async def test_input_required_is_forwarded_and_persisted( async def test_remote_error_is_posted_as_error_event( self, adapter: A2AAdapter ) -> None: - """A remote A2A outage must surface in the room, not crash the turn.""" + """A remote A2A outage must surface in the room and fail the turn.""" adapter._client = MagicMock() adapter._client.send_message = MagicMock( side_effect=RuntimeError("remote down") ) tools = FakeAgentTools() - await adapter.on_message( - make_platform_message(), - tools, - A2ASessionState(), - None, - None, - is_session_bootstrap=False, - room_id="room-123", - ) + with pytest.raises(RuntimeError, match="remote down"): + await adapter.on_message( + make_platform_message(), + tools, + A2ASessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-123", + ) - assert tools.events_sent[-1]["message_type"] == "error" - assert "remote down" in tools.events_sent[-1]["content"] + failures = reported_failures(tools) + assert failures[-1]["provider"] == "a2a" + assert failures[-1]["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE + + @pytest.mark.asyncio + async def test_on_message_reraises_delivery_failure_without_reporting_it( + self, adapter: A2AAdapter + ) -> None: + """A Band-side post failure must fail the turn for retry, without + being reported as an A2A provider failure.""" + adapter._client = MagicMock() + + async def _events() -> AsyncIterator[StreamResponse]: + yield task_event(make_task(artifact_text="Final response")) + + adapter._client.send_message = MagicMock(return_value=_events()) + tools = FakeAgentTools() + tools.send_message = AsyncMock(side_effect=RuntimeError("Band unavailable")) + + with pytest.raises(RuntimeError, match="Band unavailable"): + await adapter.on_message( + make_platform_message(), + tools, + A2ASessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-123", + ) + + assert not reported_failures(tools) @pytest.mark.asyncio async def test_failed_task_is_posted_as_error_event( @@ -390,20 +449,22 @@ async def test_failed_task_is_posted_as_error_event( ) -> None: tools = FakeAgentTools() - await adapter._handle_event( - task_event(make_task(TaskState.TASK_STATE_FAILED, status_message="boom")), - tools, - "room-123", - "user-456", - "Test User", - ) + with pytest.raises(TurnResultAlreadyReported): + await adapter._handle_event( + task_event( + make_task(TaskState.TASK_STATE_FAILED, status_message="boom") + ), + tools, + "room-123", + "user-456", + "Test User", + ) - error_events = [ - event for event in tools.events_sent if event["message_type"] == "error" - ] - assert error_events, "a failed task must produce an error event" - assert error_events[-1]["content"] == "boom" - assert error_events[-1]["metadata"]["a2a_state"] == "TASK_STATE_FAILED" + failures = reported_failures(tools) + assert failures, "a failed task must produce an error event" + assert failures[-1]["message"] == "boom" + assert failures[-1]["provider"] == "a2a" + assert failures[-1]["code"] == "TASK_STATE_FAILED" @pytest.mark.asyncio async def test_working_status_text_is_narrated_as_thought( @@ -427,6 +488,37 @@ async def test_working_status_text_is_narrated_as_thought( assert tools.events_sent[-1]["message_type"] == "thought" assert tools.events_sent[-1]["content"] == "Checking sources" + @pytest.mark.asyncio + async def test_working_status_delivery_failure_is_not_a_provider_failure( + self, adapter: A2AAdapter + ) -> None: + """A failed progress post must not blame a healthy A2A peer.""" + adapter._client = MagicMock() + adapter._client.send_message = MagicMock( + return_value=stream( + task_event( + make_task( + TaskState.TASK_STATE_WORKING, + status_message="Checking sources", + ) + ) + ) + ) + tools = FakeAgentTools() + tools.send_event_error = RuntimeError("Band unavailable") + + await adapter.on_message( + make_platform_message(), + tools, + A2ASessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-123", + ) + + assert not reported_failures(tools) + @pytest.mark.asyncio async def test_second_turn_carries_the_stored_context( self, adapter: A2AAdapter diff --git a/tests/integrations/acp/acp_toolkit/harness.py b/tests/integrations/acp/acp_toolkit/harness.py index 1b914eda0..c727aa8bc 100644 --- a/tests/integrations/acp/acp_toolkit/harness.py +++ b/tests/integrations/acp/acp_toolkit/harness.py @@ -214,6 +214,23 @@ class AcpSession: def __init__(self, adapter: ACPClientAdapter, agent: FakeACPAgent) -> None: self.adapter = adapter self.agent = agent + self._last_tools: TranscriptTools | None = None + + @property + def last_reply(self) -> Reply: + """What the most recent ``send`` posted, even if it raised. + + A genuine provider failure now fails the turn (raises out of + ``on_message``) instead of returning normally, so a test covering + that path can't get the posted error event from ``send``'s return + value — it reads this instead. + """ + assert self._last_tools is not None, "send() has not been called yet" + return Reply( + messages=self._last_tools.messages_sent, + events=self._last_tools.events_sent, + transcript=self._last_tools.transcript, + ) async def send( self, @@ -238,6 +255,7 @@ async def send( tools = TranscriptTools() if room_context is not None: tools.set_room_context(room_context) + self._last_tools = tools await self.adapter.on_message( _message(content, room), tools, diff --git a/tests/integrations/acp/test_client_adapter.py b/tests/integrations/acp/test_client_adapter.py index 8e912665e..749ad48d5 100644 --- a/tests/integrations/acp/test_client_adapter.py +++ b/tests/integrations/acp/test_client_adapter.py @@ -7,9 +7,11 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from acp.exceptions import RequestError from acp.helpers import update_agent_message_text from band.converters.parsing import parse_tool_call, parse_tool_result +from band.core.protocols import GENERIC_PROVIDER_FAILURE_MESSAGE from band.core.types import Capability from band.integrations.acp.client_adapter import ACPClientAdapter, _resolve_launcher from band.integrations.acp.client_profiles import CursorACPClientProfile @@ -20,7 +22,7 @@ ) from band.integrations.acp.room_emitter import turn_replied_in_room from band.integrations.acp.types import ACPToolCall, ACPToolResult, CollectedChunk -from band.testing import FakeAgentTools +from band.testing import FakeAgentTools, events_of_type, reported_failures from tests.integrations.acp.conftest import make_platform_message @@ -39,11 +41,6 @@ def event_types(events: list[dict[str, object]]) -> list[object]: return [event["message_type"] for event in events] -def events_of_type(tools: FakeAgentTools, message_type: str) -> list[dict[str, object]]: - """Events the handler sent, filtered to one message_type.""" - return [e for e in tools.events_sent if e.get("message_type") == message_type] - - def metadata_values(events: list[dict[str, object]], key: str) -> list[object]: """The ordered value of one metadata field across a set of events.""" return [event["metadata"][key] for event in events] @@ -745,7 +742,7 @@ async def prompt_new_session(**kwargs): async def test_on_message_error_sends_error_event( self, adapter_with_mocks: ACPClientAdapter ) -> None: - """Should send error event when ACP agent fails.""" + """Should report an AgentFailure when the ACP agent fails.""" adapter_with_mocks._runtime._conn.prompt = AsyncMock( side_effect=RuntimeError("Agent crashed") ) @@ -753,19 +750,106 @@ async def test_on_message_error_sends_error_event( tools = FakeAgentTools() msg = make_platform_message("Hello", room_id="room-123") - await adapter_with_mocks.on_message( - msg, - tools, - ACPClientSessionState(), - None, - None, - is_session_bootstrap=False, - room_id="room-123", + with pytest.raises(RuntimeError, match="Agent crashed"): + await adapter_with_mocks.on_message( + msg, + tools, + ACPClientSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-123", + ) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "acp" + assert failures[0]["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE + + @pytest.mark.asyncio + async def test_prompt_timeout_error_is_not_reported_as_adapter_timeout( + self, adapter_with_mocks: ACPClientAdapter + ) -> None: + """A provider-raised TimeoutError is not the adapter's deadline.""" + adapter_with_mocks._runtime._conn.prompt = AsyncMock( + side_effect=TimeoutError("provider socket timeout") + ) + + tools = FakeAgentTools() + + with pytest.raises(TimeoutError, match="provider socket timeout"): + await adapter_with_mocks.on_message( + make_platform_message("Hello", room_id="room-123"), + tools, + ACPClientSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-123", + ) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["message"] == GENERIC_PROVIDER_FAILURE_MESSAGE + assert failures[0]["code"] is None + + @pytest.mark.asyncio + async def test_adapter_deadline_raises_already_reported_failure( + self, adapter_with_mocks: ACPClientAdapter + ) -> None: + """The adapter's own deadline reports once and remains retryable.""" + adapter_with_mocks._turn_timeout_s = 0.01 + + async def slow_prompt(**_: object) -> None: + await asyncio.sleep(1) + + adapter_with_mocks._runtime._conn.prompt = AsyncMock(side_effect=slow_prompt) + + tools = FakeAgentTools() + + with pytest.raises(TimeoutError): + await adapter_with_mocks.on_message( + make_platform_message("Hello", room_id="room-123"), + tools, + ACPClientSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-123", + ) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["code"] == "timeout" + + @pytest.mark.asyncio + async def test_on_message_request_error_captures_code_and_data( + self, adapter_with_mocks: ACPClientAdapter + ) -> None: + """A JSON-RPC RequestError's code/data survive into the AgentFailure.""" + adapter_with_mocks._runtime._conn.prompt = AsyncMock( + side_effect=RequestError(-32603, "Internal error", {"detail": "oom"}) ) - error_events = events_of_type(tools, "error") - assert len(error_events) == 1 - assert "Agent crashed" in error_events[0]["content"] + tools = FakeAgentTools() + msg = make_platform_message("Hello", room_id="room-123") + + with pytest.raises(RequestError): + await adapter_with_mocks.on_message( + msg, + tools, + ACPClientSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-123", + ) + + failures = reported_failures(tools) + assert len(failures) == 1 + assert failures[0]["provider"] == "acp" + assert failures[0]["code"] == "-32603" + assert failures[0]["detail"] == {"detail": "oom"} @pytest.mark.asyncio async def test_on_message_not_initialized_raises(self) -> None: @@ -1311,6 +1395,91 @@ async def test_prompt_error_clears_connection(self) -> None: tools = FakeAgentTools() msg = make_platform_message("Hello", room_id="room-1") + with pytest.raises(RuntimeError, match="Process died"): + await adapter.on_message( + msg, + tools, + ACPClientSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) + + # Connection should be cleared after error + assert adapter._runtime._conn is None + assert adapter._runtime._ctx is None + + # AgentFailure should be reported + assert len(reported_failures(tools)) == 1 + + @pytest.mark.asyncio + async def test_reply_delivery_failure_leaves_connection_up(self) -> None: + """The agent answered fine; posting its reply to the room is what + failed. That must not tear down and respawn a healthy connection, + nor be reported as an ACP provider failure.""" + adapter = ACPClientAdapter(command="codex", inject_band_tools=False) + adapter._runtime._conn = AsyncMock() + mock_session = MagicMock() + mock_session.session_id = "sess-1" + adapter._runtime._conn.new_session = AsyncMock(return_value=mock_session) + adapter._runtime._client = BandACPClient() + + mock_ctx = MagicMock() + mock_ctx.__aexit__ = AsyncMock(return_value=None) + adapter._runtime._ctx = mock_ctx + + async def prompt_with_reply(**kwargs): + session_id = kwargs["session_id"] + await adapter._runtime._client.session_update( + session_id, update_agent_message_text("Here's the answer") + ) + + adapter._runtime._conn.prompt = AsyncMock(side_effect=prompt_with_reply) + + tools = FakeAgentTools() + + async def _raise(*args: object, **kwargs: object) -> None: + raise RuntimeError("platform rejected the message") + + tools.send_message = _raise # type: ignore[method-assign] + + msg = make_platform_message("Hello", room_id="room-1") + + with pytest.raises(RuntimeError, match="platform rejected the message"): + await adapter.on_message( + msg, + tools, + ACPClientSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-1", + ) + + assert adapter._runtime._conn is not None + assert adapter._runtime._ctx is not None + assert not reported_failures(tools) + + @pytest.mark.asyncio + async def test_session_bookkeeping_failure_leaves_connection_up(self) -> None: + """A failed session task event must not turn a completed prompt into an ACP failure.""" + adapter = ACPClientAdapter(command="codex", inject_band_tools=False) + adapter._runtime._conn = AsyncMock() + mock_session = MagicMock() + mock_session.session_id = "sess-1" + adapter._runtime._conn.new_session = AsyncMock(return_value=mock_session) + adapter._runtime._client = BandACPClient() + + mock_ctx = MagicMock() + mock_ctx.__aexit__ = AsyncMock(return_value=None) + adapter._runtime._ctx = mock_ctx + + adapter._runtime._conn.prompt = AsyncMock() + tools = FakeAgentTools() + tools.send_event_error = RuntimeError("platform rejected the task event") + msg = make_platform_message("Hello", room_id="room-1") + await adapter.on_message( msg, tools, @@ -1321,13 +1490,81 @@ async def test_prompt_error_clears_connection(self) -> None: room_id="room-1", ) - # Connection should be cleared after error - assert adapter._runtime._conn is None - assert adapter._runtime._ctx is None + assert adapter._runtime._conn is not None + assert adapter._runtime._ctx is not None + assert not reported_failures(tools) + + @pytest.mark.asyncio + async def test_turn_timeout_preserves_other_room_connection(self) -> None: + """A timed-out room must not interrupt another room's prompt.""" + adapter = ACPClientAdapter( + command="codex", inject_band_tools=False, turn_timeout_s=1 + ) + adapter._runtime._conn = AsyncMock() + session_b = MagicMock(session_id="sess-b") + session_a = MagicMock(session_id="sess-a") + adapter._runtime._conn.new_session = AsyncMock( + side_effect=[session_b, session_a] + ) + adapter._runtime._client = BandACPClient() + + mock_ctx = MagicMock() + mock_ctx.__aexit__ = AsyncMock(return_value=None) + adapter._runtime._ctx = mock_ctx + + b_started = asyncio.Event() + release_b = asyncio.Event() + + async def prompt(*, session_id: str, **kwargs: object) -> None: + if session_id == "sess-b": + b_started.set() + await release_b.wait() + else: + await asyncio.sleep(10) + + adapter._runtime._conn.prompt = AsyncMock(side_effect=prompt) + + tools_b = FakeAgentTools() + b_turn = asyncio.create_task( + adapter.on_message( + make_platform_message("Hello", room_id="room-b"), + tools_b, + ACPClientSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-b", + ) + ) + await b_started.wait() + adapter._turn_timeout_s = 0.01 + + tools_a = FakeAgentTools() + + with pytest.raises(TimeoutError): + await adapter.on_message( + make_platform_message("Hello", room_id="room-a"), + tools_a, + ACPClientSessionState(), + None, + None, + is_session_bootstrap=False, + room_id="room-a", + ) - # Error event should be sent - error_events = events_of_type(tools, "error") - assert len(error_events) == 1 + assert not b_turn.done() + assert adapter._runtime._conn is not None + assert adapter._runtime._ctx is not None + assert "room-a" not in adapter._room_to_session + assert adapter._room_to_session["room-b"] == "sess-b" + adapter._runtime._conn.cancel.assert_awaited_once_with("sess-a") + failures = reported_failures(tools_a) + assert len(failures) == 1 + assert failures[0]["provider"] == "acp" + assert failures[0]["code"] == "timeout" + + release_b.set() + await b_turn class TestACPClientAdapterInjectToolsConfig: diff --git a/tests/integrations/acp/test_client_adapter_behavior.py b/tests/integrations/acp/test_client_adapter_behavior.py index 5004a71a2..182a59e50 100644 --- a/tests/integrations/acp/test_client_adapter_behavior.py +++ b/tests/integrations/acp/test_client_adapter_behavior.py @@ -813,7 +813,9 @@ async def _script(a: FakeACPAgent, sid: str) -> None: async with acp_adapter(agent) as session: await session.send("My favorite color is blue.", bootstrap=True) - crashed = await session.send("anything") # prompt raises -> adapter stop() + with pytest.raises(RequestError): + await session.send("anything") # prompt raises -> adapter stop() + crashed = session.last_reply reply = await session.send( "What is my favorite color?", room_context=transcript ) diff --git a/tests/integrations/claude_sdk/test_dedup_tools.py b/tests/integrations/claude_sdk/test_dedup_tools.py index 86954f5d4..fcc60e300 100644 --- a/tests/integrations/claude_sdk/test_dedup_tools.py +++ b/tests/integrations/claude_sdk/test_dedup_tools.py @@ -7,6 +7,7 @@ from unittest.mock import AsyncMock, MagicMock import pytest +from band_sdk_core import AgentFailure from band.integrations.claude_sdk.dedup_tools import ( DEFAULT_DEDUP_MAX_ENTRIES, @@ -20,6 +21,7 @@ def _make_inner() -> MagicMock: inner = MagicMock() inner.send_message = AsyncMock(return_value={"id": "msg-1"}) inner.send_event = AsyncMock(return_value={"id": "evt-1"}) + inner.send_failure = AsyncMock(return_value={"id": "evt-2"}) inner.add_participant = AsyncMock(return_value={"id": "u"}) inner.participants = ["p1", "p2"] return inner @@ -292,6 +294,18 @@ async def test_other_methods_forward_unchanged(self): ) inner.add_participant.assert_awaited_once_with("@svc/bot") + @pytest.mark.asyncio + async def test_send_failure_forwards_unchanged(self): + """No dedup special-casing for send_failure -- __getattr__ forwards + it straight through, same as every other AgentToolsProtocol method.""" + inner = _make_inner() + wrapper = DedupingAgentTools(inner) + failure = AgentFailure("codex", "boom") + + await wrapper.send_failure(failure) + + inner.send_failure.assert_awaited_once_with(failure) + def test_attributes_forward_unchanged(self): inner = _make_inner() wrapper = DedupingAgentTools(inner) diff --git a/tests/integrations/parlant/test_tools.py b/tests/integrations/parlant/test_tools.py index 60872a5db..07b1ae846 100644 --- a/tests/integrations/parlant/test_tools.py +++ b/tests/integrations/parlant/test_tools.py @@ -500,6 +500,7 @@ def mock_tools(self): tools = MagicMock() tools.send_message = AsyncMock() tools.send_event = AsyncMock() + tools.send_failure = AsyncMock() tools.add_participant = AsyncMock(return_value={"status": "added"}) tools.remove_participant = AsyncMock() tools.lookup_peers = AsyncMock( diff --git a/tests/runtime/test_tools.py b/tests/runtime/test_tools.py index 8f49f8442..badac4e31 100644 --- a/tests/runtime/test_tools.py +++ b/tests/runtime/test_tools.py @@ -14,6 +14,7 @@ GetAgentChatContextResponse, GetAgentChatContextResponseMetadata, ) +from band_sdk_core import AgentFailure from pydantic import BaseModel, ValidationError from band.client.rest import ( @@ -1489,6 +1490,50 @@ async def test_send_event_refuses_content_with_no_visible_characters( mock_rest_client.agent_api_events.create_agent_chat_event.assert_not_called() +class TestAgentToolsSendFailure: + """Test send_failure's best-effort delegation over the real REST boundary.""" + + async def test_send_failure_posts_an_error_event(self, mock_rest_client): + tools = AgentTools("room-123", mock_rest_client) + + await tools.send_failure(AgentFailure("codex", "boom", "timeout")) + + call_args = mock_rest_client.agent_api_events.create_agent_chat_event.call_args + event = call_args.kwargs["event"] + assert event.message_type == "error" + assert event.metadata["failure"] == { + "provider": "codex", + "code": "timeout", + "message": "boom", + "detail": None, + } + + async def test_send_failure_swallows_a_rest_rejection(self, mock_rest_client): + """A failed report must resolve, not raise -- it runs inside a + caller's except block reporting a real provider failure already.""" + mock_rest_client.agent_api_events.create_agent_chat_event.side_effect = ( + RuntimeError("REST rejected the event") + ) + tools = AgentTools("room-123", mock_rest_client) + + result = await tools.send_failure(AgentFailure("codex", "boom")) + + assert result == {"ok": False, "error": "REST rejected the event"} + + async def test_send_event_itself_still_raises_on_the_same_rejection( + self, mock_rest_client + ): + """The other half of the best-effort contract: send_event's own + raising behavior is unchanged by send_failure wrapping it.""" + mock_rest_client.agent_api_events.create_agent_chat_event.side_effect = ( + RuntimeError("REST rejected the event") + ) + tools = AgentTools("room-123", mock_rest_client) + + with pytest.raises(RuntimeError, match="REST rejected the event"): + await tools.send_event("task update", "task") + + class TestMatchesIdentifier: """Tests for the _matches_identifier helper.""" diff --git a/tests/testing/test_fake_tools.py b/tests/testing/test_fake_tools.py index 5361188e2..86918b048 100644 --- a/tests/testing/test_fake_tools.py +++ b/tests/testing/test_fake_tools.py @@ -5,11 +5,12 @@ from typing import Any import pytest +from band_sdk_core import AgentFailure from band.core.exceptions import BandToolError from band.core.protocols import AgentToolsProtocol from band.runtime.tools import DEFAULT_FILE_CAPTION, serialize_tool_result -from band.testing import FakeAgentTools +from band.testing import FakeAgentTools, reported_failures from tests.content import BLANK_CONTENT_CASES @@ -236,6 +237,85 @@ async def test_refuses_content_with_no_visible_characters(self, content): assert tools.events_sent == [] +class TestSendFailure: + """Tests for send_failure's best-effort delegation to send_event.""" + + async def test_posts_an_error_event_carrying_the_failure(self): + tools = FakeAgentTools() + + result = await tools.send_failure(AgentFailure("codex", "boom", "timeout")) + + assert len(tools.events_sent) == 1 + assert tools.events_sent[0]["message_type"] == "error" + assert tools.events_sent[0]["metadata"]["failure"] == { + "provider": "codex", + "code": "timeout", + "message": "boom", + "detail": None, + } + assert result["message_type"] == "error" + + async def test_blank_message_falls_back_to_a_generic_one(self): + tools = FakeAgentTools() + + await tools.send_failure(AgentFailure("acp", "")) + + assert tools.events_sent[0]["content"] == "acp failed without an error message." + + async def test_swallows_a_send_event_failure_instead_of_raising(self): + """send_failure must resolve even when the underlying report fails -- + raising here would replace the provider failure being reported with + an unrelated one.""" + tools = FakeAgentTools() + tools.send_event_error = RuntimeError("platform rejected the event") + + result = await tools.send_failure(AgentFailure("codex", "boom")) + + assert result == {"ok": False, "error": "platform rejected the event"} + assert tools.events_sent == [] + + async def test_send_event_itself_still_raises_on_the_same_failure(self): + """The other half of the best-effort contract: send_event's raising + callers (e.g. OpenCode's session-persistence retry) must be + unaffected by send_failure's swallowing.""" + tools = FakeAgentTools() + tools.send_event_error = RuntimeError("platform rejected the event") + + with pytest.raises(RuntimeError, match="platform rejected the event"): + await tools.send_event("task update", "task") + + +class TestReportedFailures: + """reported_failures() is the shared projection every migrated adapter's + test suite uses to assert on a reported AgentFailure -- a regression here + would silently weaken failure assertions across the whole test suite.""" + + async def test_returns_each_reported_failure_in_order(self): + tools = FakeAgentTools() + + await tools.send_failure(AgentFailure("codex", "first")) + await tools.send_failure(AgentFailure("letta", "second")) + + failures = reported_failures(tools) + + assert [f["message"] for f in failures] == ["first", "second"] + assert [f["provider"] for f in failures] == ["codex", "letta"] + + async def test_ignores_a_plain_error_event_with_no_failure_metadata(self): + """A pre-migration send_event(..., "error") call carries no + ``failure`` metadata -- it must not be mistaken for a reported + AgentFailure.""" + tools = FakeAgentTools() + + await tools.send_event("legacy error text", "error") + await tools.send_failure(AgentFailure("codex", "structured failure")) + + failures = reported_failures(tools) + + assert len(failures) == 1 + assert failures[0]["message"] == "structured failure" + + class TestParticipantOperations: """Tests for participant tracking.""" diff --git a/uv.lock b/uv.lock index ab9022a0a..9caf06778 100644 --- a/uv.lock +++ b/uv.lock @@ -780,7 +780,7 @@ requires-dist = [ { name = "anthropic", marker = "extra == 'dev-parlant'", specifier = ">=0.75.0,<1" }, { name = "async-lru", specifier = ">=2.3.0" }, { name = "band-client-rest", specifier = "==0.0.27" }, - { name = "band-sdk-core", specifier = "==2.2.0" }, + { name = "band-sdk-core", specifier = "==2.3.0" }, { name = "band-testing-python", marker = "extra == 'dev'", specifier = "==0.1.4" }, { name = "band-testing-python", marker = "extra == 'dev-crewai'", specifier = "==0.1.4" }, { name = "band-testing-python", marker = "extra == 'dev-parlant'", specifier = "==0.1.4" }, @@ -925,17 +925,17 @@ provides-extras = ["logging", "desktop", "codex", "opencode", "letta", "pydantic [[package]] name = "band-sdk-core" -version = "2.2.0" +version = "2.3.0" source = { registry = "https://pypi.org/simple" } wheels = [ - { url = "https://files.pythonhosted.org/packages/f2/b6/6432cd80745c68517ffb2efb9f8dc5f5d2ad423eaccb002d7a14dd1d28f9/band_sdk_core-2.2.0-cp311-abi3-macosx_10_12_x86_64.whl", hash = "sha256:2eaa8f62a5b806d10993ea4feb73f1c48d3c959b9539bfbcdbd1ffa1f426b998", size = 475871, upload-time = "2026-09-01T11:30:26.358Z" }, - { url = "https://files.pythonhosted.org/packages/9a/1d/a20cbfce00fd32ec5e1145528138fd2b0d8af12dd09c337a59c6c4b79f69/band_sdk_core-2.2.0-cp311-abi3-macosx_11_0_arm64.whl", hash = "sha256:52c4c6af089e81d877e34b839694e8b01547d0f25891cf1bc9c717cf32278f59", size = 477493, upload-time = "2026-09-01T11:30:27.934Z" }, - { url = "https://files.pythonhosted.org/packages/e5/d8/4ec2c9a0b37027072e92dbe2912fce0d593ee88c3f96e1281882429588ff/band_sdk_core-2.2.0-cp311-abi3-manylinux_2_28_aarch64.whl", hash = "sha256:9c99fad0f3edb0fc9a9d42079a2be76d573c4dd7a9579ce4b5aea1d5fabaaf9e", size = 526652, upload-time = "2026-09-01T11:30:29.398Z" }, - { url = "https://files.pythonhosted.org/packages/61/1a/548e4f355517ee7cd4422906055765b955ec543efc95ef4993ac8a2a84ed/band_sdk_core-2.2.0-cp311-abi3-manylinux_2_28_x86_64.whl", hash = "sha256:431e7185d2a8659371a6fb22aa0a3383e935607d1c8944f488eebf70b0821cf7", size = 528152, upload-time = "2026-09-01T11:30:30.953Z" }, - { url = "https://files.pythonhosted.org/packages/37/9a/6b19bf76fc55d4f5a1315ac036506aea71c499651b0fbe328004c9e40011/band_sdk_core-2.2.0-cp311-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:1996fb61fb22d5750df214b3d180e183cdbf3f7e36cea960ca8ca2442a93b888", size = 705290, upload-time = "2026-09-01T11:30:32.514Z" }, - { url = "https://files.pythonhosted.org/packages/c7/8a/5bceb2a5732fb51b1fa62d6dea172104ea79080c768a0aa4a4d42ddf5d5c/band_sdk_core-2.2.0-cp311-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:8688f4c55c7cd778e5bb2846b6ada03e7c392d9eb8565a698fdb3df6e29faa8b", size = 743509, upload-time = "2026-09-01T11:30:34.036Z" }, - { url = "https://files.pythonhosted.org/packages/0e/00/c267b0fc208f121ad25b41f41a18c5d77c9280049eae8a359821d37c2a33/band_sdk_core-2.2.0-cp311-abi3-win_amd64.whl", hash = "sha256:704bff82b7494f1997df1aa20def09d7431075f0ad0a1dd4895a4b916409a9f2", size = 348322, upload-time = "2026-09-01T11:30:35.664Z" }, - { url = "https://files.pythonhosted.org/packages/79/d5/46a9dce9b20509904d69b181685fd2d1d498c657466dcd58ba0fedb78248/band_sdk_core-2.2.0-cp311-abi3-win_arm64.whl", hash = "sha256:7f49e75f6f890f38ba7366fb9264c8ca01c34f231db7ab8f08f4c6cc9c161295", size = 331334, upload-time = "2026-09-01T11:30:36.826Z" }, + { url = "https://files.pythonhosted.org/packages/af/f4/25f7be88054b13dbba887eee2dedcbf50c33e46cb4b4d14b71359b7fcb0d/band_sdk_core-2.3.0-cp311-abi3-macosx_10_12_x86_64.whl", hash = "sha256:9a12f9a447a684b50eccf8fb779928938f72edac02b11bc1b0da67597ff1daa1", size = 478061, upload-time = "2026-09-06T05:54:16.834Z" }, + { url = "https://files.pythonhosted.org/packages/5d/0d/7adc5f3cb9b001ead33ea829932958bb0fff8f5d543a891d1338b94f3ea4/band_sdk_core-2.3.0-cp311-abi3-macosx_11_0_arm64.whl", hash = "sha256:4b5232055f48da4d788d83502d9e14453c6153d41c4e9750133f960a6fd18f4d", size = 479953, upload-time = "2026-09-06T05:54:18.464Z" }, + { url = "https://files.pythonhosted.org/packages/45/ab/14449d8967d2fcde4d84455070f5e1d011d9b2f76b22383af97971d20134/band_sdk_core-2.3.0-cp311-abi3-manylinux_2_28_aarch64.whl", hash = "sha256:ee15965fc503b372adc6965999acb26dc67cac4d92d7fe7d876b6786b8ae73e4", size = 531154, upload-time = "2026-09-06T05:54:19.997Z" }, + { url = "https://files.pythonhosted.org/packages/5e/ff/5de82d4e5d72436039be905de68436ae5689ac44892214fbca652b0b2f30/band_sdk_core-2.3.0-cp311-abi3-manylinux_2_28_x86_64.whl", hash = "sha256:044769078b6e7ed28062f8280c269e4e904e700c4112f791913ba48d0f255449", size = 530174, upload-time = "2026-09-06T05:54:21.272Z" }, + { url = "https://files.pythonhosted.org/packages/22/b4/cfe7d0d8eaad24c975f4bc6e9d29634225caee6537c1be197a35cc3b0d8a/band_sdk_core-2.3.0-cp311-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:4d44b363199fb5bb7b3a21d26c70d443ed381c923cd48f0df2f20f690ac79e23", size = 710040, upload-time = "2026-09-06T05:54:22.7Z" }, + { url = "https://files.pythonhosted.org/packages/17/f2/7758410757818756395449a653f4ba44098af104612eca478c585ec36f59/band_sdk_core-2.3.0-cp311-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:eb541d6c12437e4f12cee4b8db2b5674d8414a8ae018e92267ec237bb1302599", size = 746399, upload-time = "2026-09-06T05:54:24.02Z" }, + { url = "https://files.pythonhosted.org/packages/f4/72/6de9b5820723c04f41ea2644cda496857f0e3151418bf090b80e750f7734/band_sdk_core-2.3.0-cp311-abi3-win_amd64.whl", hash = "sha256:ad2df3ff7ab79d06fe17dd970b95fd9f19c38687368d871bd3e3ab7a328de1de", size = 351424, upload-time = "2026-09-06T05:54:25.483Z" }, + { url = "https://files.pythonhosted.org/packages/b7/8e/cdf700c5553dd0ba879f190b02774b9dff64c7d3438f0acfcb5212477509/band_sdk_core-2.3.0-cp311-abi3-win_arm64.whl", hash = "sha256:adbbdb9cf318bd69e369a02f4fafa7323c2202b903acaaae7ef91175ed61faa1", size = 333893, upload-time = "2026-09-06T05:54:26.84Z" }, ] [[package]]