feat: Adopt AgentFailure/send_failure across Python adapters - #612
Open
AlexanderZ-Band wants to merge 28 commits into
Open
feat: Adopt AgentFailure/send_failure across Python adapters#612AlexanderZ-Band wants to merge 28 commits into
AlexanderZ-Band wants to merge 28 commits into
Conversation
The AgentFailure shared failure shape shipped in this release via release-please automation, unblocking the send_failure migration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Adds the shared, provider-neutral AgentFailure surfacing contract: send_failure on AgentToolsProtocol/AgentTools/FakeAgentTools, and the to_failure_event helper both implementations delegate to. send_failure is best-effort (swallows its own reporting failure); send_event keeps its existing raising behavior unchanged, since stateful callers (e.g. OpenCode's session-persistence retry) depend on it as a control signal. FakeAgentTools gains a send_event_error hook so tests can simulate a REST rejection without touching send_message's own simulation path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
A Band-side send_message rejection must never be reported as a provider failure, even when the send sits inside a shared try/except that also handles real provider errors. deliver_reply wraps the cause so a catch site can re-raise it before falling into its provider-failure branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Replaces the ad hoc "error" event/_report_error guard with send_failure(AgentFailure(...)), preserving APIStatusError's status_code/body as code/detail (or falling back to a generic failure for any other exception type). No change to the existing raise-after-report control flow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Replaces the ad hoc "error" event/_report_error guard with send_failure(AgentFailure(...)), preserving ServerError's status/message as code/detail (generic fallback otherwise). Also closes a gap where exceeding max_tool_rounds raised RuntimeError with zero report at all. No change to the existing raise-after-report control flow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Replaces the ad hoc "error" event/_report_error guard with send_failure(AgentFailure(...)). Also widens the try to cover per-message runner construction (_create_runner), which previously ran outside the try entirely and could fail with zero report; the runner now starts as None so the existing finally's close() has nothing to do if construction itself is what failed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Replaces the ad hoc "error" event/_report_error guard with send_failure(AgentFailure(...)), preserving ResultMessage's api_error_status/errors as code/detail. Closes two previously-silent gaps: a bare re-raise in the session resume-retry path when there is no stored session to fall back to, and the fallback session-creation attempt itself failing. DedupingAgentTools needs no change (forwards send_failure via __getattr__ like every other method). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Every hand-rolled MagicMock() AgentToolsProtocol double that sets send_event as an AsyncMock needs the same for send_failure, or the next adapter migrated onto it fails with "MagicMock can't be awaited" instead of a real assertion. Fixing the remaining fixtures now so each adapter's own migration commit doesn't have to rediscover this. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Replaces the ad hoc "error" event/_report_error guard with send_failure(AgentFailure(...)). Also widens coverage to _obtain_session (create/resume session setup), which previously ran outside on_message's try entirely and could fail with zero report. _send_event_safe stays untouched -- its other callers depend on its boolean return to drive session-persistence retry. Copilot ACP needs no separate change: it's a pure config subclass of ACPClientAdapter and inherits that adapter's fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Replaces the ad hoc "error" event with send_failure(AgentFailure(...)), preserving the existing redaction guarantee: exception text can carry DB strings, paths, and tokens, so message stays a fixed generic string and code/detail stay unset -- never populated from the caught exception. Also widens the try to cover graph-factory construction, which previously ran outside it entirely and could fail with zero report. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Replaces the ad hoc "error" event/_report_error guard with send_failure(AgentFailure(...)) across all report-and-return call sites, unchanged control flow. Routes the auto-relay reply through deliver_reply so a Band-side send_message rejection surfaces as a delivery failure rather than a misclassified Letta provider failure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
AlexanderZ-Band
changed the title
AlexanderZ-Band
marked this pull request as draft
September 6, 2026 11:55
Replaces the ad hoc "error" event with send_failure(AgentFailure(...)), preserving the task's state_name(state) as code for a terminal failure state. Routes both room-reply and task-update send_message calls through deliver_reply so a Band-side delivery rejection is never misclassified as an A2A provider failure -- the existing report-and-return control flow is otherwise unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Report AgentFailure for the not-initialized guard, the missing-reply guard, and the generic turn exception, replacing the ad hoc _report_error helper (now dead and removed). Also tidies a stale comment in the Claude SDK test file left over from that adapter's own migration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
record_failed's visibility event now reports an AgentFailure instead of a hand-built send_event error payload, matching every other migrated adapter. The task-status event's own embedded error field (a distinct, flow-internal envelope) is unrelated and untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
The turn's run loop had no catch-all: only UnexpectedModelBehavior was handled, and its genuine-failure branch (as well as every other exception type) propagated with zero report — the worst gap in the adapter survey. Consolidate into one except Exception, mirroring the CrewAI adapter's swallow-then-report structure, so every exception that isn't the benign post-reply output-retry exhaustion now reports an AgentFailure before propagating. Also migrates the missing-reply guard off the deleted _report_error helper. The band_respond_contact_request tool handler's own error event is left untouched: that failure originates from a Band platform tool call, not the pydantic_ai provider, so it is out of scope for AgentFailure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Migrates the session-init and generic-turn-error reports off the deleted _report_error helper, and adds a report to the uninitialized-app guard, which previously returned silently with zero report. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
_run_turn's agent construction + invoke_async had no except clause at all, so any provider failure propagated with zero report — the worst gap in this adapter. Wrap both in one except Exception that reports before reraising, and migrate the missing-reply guard off the deleted _report_error helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
_run_agent's shared except now reports an AgentFailure instead of a raw send_event: message stays a fixed, redacted string (Agno's swallowed run/exception text can carry DB strings, paths, or tokens), and code is set to RunStatus.error's value only when the exception is an AgnoRunError -- never populated from response.content. Also widens the reported boundary to the agent-null guard and _build_run_input, both previously unguarded ahead of _run_agent's try. Investigated but did not build the contextvar-based delivery-failure slot the plan called for: band_send_message failures never reach here as a raised exception. execute_tool_call_structured (agent.py) catches every non-BandToolError exception and returns it as a plain string result, and send_message's own delivery path (post_message) never raises BandToolError -- so a Band-side delivery failure can't reach _run_agent's except at all, let alone get misclassified as an AgnoRunError. Verified against the installed agno package's own run-loop and function-call exception handling. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Migrates 4 of the adapter's 8 "error" sites to AgentFailure: the HTTP error branch (preserving the status code), the generic turn-failure fallback, the turn-timeout report (code="timeout"), and the terminal last_error_message delivered at end of turn. The other 4 stay send_event: the still-processing backpressure guard, the delivery -failure notice, and the two human-approval-timeout notices in approvals.py -- none of these are provider failures. Adds events_of_type/reported_failures test helpers (mirroring the existing per-package convention in the ACP and Codex test suites) and negative-assertion coverage proving the two approval-timeout sites never carry the shared failure metadata shape. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Replace the structured-error/plain-text dual path (gated by the now-deleted CodexAdapterConfig.structured_errors flag) with a single unconditional path that converts Codex's error payloads into the shared AgentFailure shape via build_agent_failure (replacing build_structured_error_metadata) and reports them with send_failure. is_retryable no longer defaults to False when the upstream codexErrorInfo omits it -- absence now stays unknown rather than lying about retryability. Reply/bookkeeping posts (_handle_local_command's slash-command replies, _emit_turn_outcome's fallback text and error text) now go through deliver_reply so a Band-side delivery failure raises DeliveryFailedError instead of being misread as a Codex provider error. on_message's turn-setup through turn-outcome span is wrapped so that boundary: DeliveryFailedError is logged and swallowed, any other exception is reported as an AgentFailure and re-raised, preserving existing raise-vs-report control flow everywhere else. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
PendingA2ATask.fail() gains an optional failure dict, attached as TaskStatusUpdateEvent metadata (via TaskUpdater.update_status, which failed() didn't expose) alongside its existing freeform reason text. Two of the five call sites are Band-side room lifecycle (room closed, gateway shut down) and stay untouched -- neither is a provider failure. The other three are provider-originated: - _execute_a2a's broad except and _await_response's timeout synthesize AgentFailure(provider="a2a-gateway", ...) for gateway-relay failures, running the exception text through a redaction/length-cap sanitizer mirroring the TS SDK's sanitizeGatewayErrorMessage before it reaches the external A2A client. - _publish_band_response's "error" message_type case relays the Band peer's own already-built AgentFailure (its adapter's send_failure already stamped metadata["failure"] via to_failure_event) unchanged, rather than re-tagging its provider as "a2a-gateway". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
on_message's single free-text "error" event becomes send_failure with a new _to_agent_failure(exc) converter, which unwraps acp.exceptions. RequestError's numeric code/data instead of collapsing a JSON-RPC error into one generic string. RoomTurnEmitter.__aexit__ relays a turn's held text through deliver_reply instead of a bare send_message call, and on_message's except now splits DeliveryFailedError from a real provider failure: a Band-side post failure is logged and left alone (the connection stays up, nothing is reported), where previously it fell into the generic branch and both tore down/respawned the ACP connection and misreported a healthy agent turn as an "ACP agent error". Left out of scope (verified against source, not present today): a turn timeout, transport-health-scoped classification of which failures should respawn the connection, and cancel_turn/stop_reason propagation. Each is a new resilience feature or a recovery-action policy change, not error- shape surfacing, and none corresponds to an existing bug in this file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
AlexanderZ-Band
marked this pull request as ready for review
September 6, 2026 13:34
- codex.py: on_message's except DeliveryFailedError now re-raises the
original cause instead of swallowing it. This call path had no
try/except at all before this PR, so any exception (delivery or
provider) used to propagate and durably mark_failed the message;
swallowing it silently turned a failed reply-delivery into a
successfully processed message with no observable trace.
- acp/client_adapter.py: adds a turn_timeout_s config (default 300s,
mirroring Letta's own field) wrapping _runtime.prompt via
asyncio.wait_for, reporting a "timeout"-coded AgentFailure and
tearing the connection down for the next turn to respawn. This is an
explicit Requirement in the INT-1385 ticket text itself ("so a
silent/stuck agent becomes an observable failure instead of hanging
indefinitely") and was mistakenly bundled into this PR's earlier note
about descoped ACP resilience features -- the ticket treats the
timeout as the minimum required surfacing fix, not an elective one.
- crewai_flow.py: record_failed's message is capped at 500 chars again,
matching every other room post in the file (record_waiting,
reply_ambiguous) and the cap the old hand-built send_event path had.
The ambiguous-participant-identity error embeds every colliding
participant id and can exceed that length in a large room.
- a2a/gateway/adapter.py: the timeout AgentFailure's code is now
lowercase "timeout", matching Letta's and OpenCode's existing
convention instead of introducing a second casing for the same
closed vocabulary.
- codex.py: _handle_approval_command's remaining send_message calls
now go through deliver_reply too, for consistency with every other
slash-command handler in the same file (this method sits outside any
try/except either way, so behavior is unchanged; this only future-
proofs it if that ever changes).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Four parallel review agents (reuse, simplification, efficiency, altitude) checked the AgentFailure migration diff for cleanup opportunities. Applied the verified, in-scope findings: - to_failure_event now uses has_visible_content() instead of a bare .strip(), matching the platform's actual blank-content rule (a zero-width-only message previously slipped past the blank check) - centralized the reported_failures() test helper (band.testing) instead of three duplicated copies plus one inlined reimplementation - send_failure implementations pass MessageType.ERROR instead of the raw "error" literal - added a shared FAILURE_CODE_TIMEOUT constant instead of four independent "timeout" literals across letta/acp/opencode/a2a-gateway - a2a-gateway's timeout branch reuses failure.message instead of retyping the same string twice - codex.py's 5 new task-event send_event calls go through one _emit_event_safe helper instead of duplicating the try/except-log shape (mirrors copilot_sdk's existing _send_event_safe) - removed a docs/adapters/codex.md row describing the now-deleted structured_errors config field Skipped: a shared SimpleAdapter provider-slug ClassVar and a shared report_failure(tools, provider, exc) helper (both touch adapter class-shape/control-flow well beyond this migration's own new code), and per-adapter exception-to-AgentFailure enrichment unification (genuinely adapter-specific, tied to each SDK's own exception types). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
Verified each finding directly against source/tests before accepting.
Credential redaction (a2a-gateway): the KV regex stopped its value group
at the first whitespace, so a scheme-prefixed credential ("Authorization:
ApiKey sk-...") only redacted up to the space and leaked the real secret;
widened it, and applied the same redaction to the peer-forwarded-failure
relay path (on_event's "error" branch), which previously reached an
external A2A client with zero redaction, unlike this adapter's own
exception path.
A2A adapter: the finally block's terminal task-event emission could
replace a DeliveryFailedError already propagating from the try block
(Python try/finally semantics), turning a Band delivery outage into a
fabricated "a2a" provider failure once it reached on_message. Now
isolated in its own try/except.
CrewAI Flow: record_failed's 500-char cap now preserves the untruncated
message in AgentFailure.detail instead of discarding it.
fake_tools.reported_failures(): now ignores "error" events with no
failure metadata, so pydantic_ai's still-unmigrated bare send_event(...,
"error") call doesn't crash it with a KeyError.
Codex: the turn-timeout path never called send_failure, unlike every
sibling adapter's timeout handling; added it. Deduped the case where both
an "error" notification and a failed turn/completed fire for the same
incident (was reporting twice). Wrapped _handle_approval_command's reply
delivery in the same DeliveryFailedError handling as every other reply
path in the file (it ran outside it). Tightened _emit_structured_turn_error's
non-dict-error guard so a falsy scalar (e.g. False) can't become the
literal string "False" in a room-visible message.
Also centralized events_of_type in band.testing (removing a dead,
byte-identical copy in tests/adapters/opencode/helpers.py and several
inline reimplementations), and fixed two comments that narrated the
diff's own history instead of stating a present-tense fact.
Added regression tests for every fix above (the review's own pass hadn't
added coverage for the codex.py, a2a/adapter.py, or a2a/gateway fixes) --
confirmed each new a2a test actually fails against the pre-fix code.
Not fixed: ACPClientAdapter's turn-timeout calls self.stop(), which tears
down the adapter-wide shared runtime, evicting every other room's
in-flight session on one room's timeout. Real, but pre-existing (the
adapter's generic except-Exception branch already did this before this
PR) and needs a per-room isolation design, not a bolt-on patch -- flagging
for a separate discussion rather than guessing at a fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CmsEQaNzmSmBGVsCTuKvbX
There was a problem hiding this comment.
🟡 Changes recommended
A2A gateway peer-failure relaying redacts failure.message but can still forward credentials embedded in failure.detail unredacted.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR standardizes adapter failure reporting across the Band Python SDK by adopting band-sdk-core’s shared AgentFailure shape and adding a best-effort send_failure(...) pathway, while separating Band-side reply delivery failures from provider/adapter failures.
Changes:
- Bumped
band-sdk-coreto2.3.0and migrated adapters/tests away from ad hoc"error"event metadata to structuredAgentFailure. - Added shared failure helpers (
FAILURE_CODE_TIMEOUT,to_failure_event) and delivery-vs-provider failure separation (DeliveryFailedError,deliver_reply). - Updated multiple adapters/integrations (notably ACP client turn timeout and A2A/A2A-gateway failure handling) plus centralized test helpers (
events_of_type,reported_failures).
File summaries
| File | Description |
|---|---|
| uv.lock | Updates lockfile for band-sdk-core==2.3.0. |
| pyproject.toml | Bumps band-sdk-core dependency pin. |
| docs/adapters/codex.md | Removes structured_errors flag from docs. |
| src/band/core/protocols.py | Adds AgentFailure, send_failure, to_failure_event, timeout code constant. |
| src/band/core/delivery.py | Introduces deliver_reply + DeliveryFailedError wrapper. |
| src/band/runtime/tools/agent.py | Implements AgentTools.send_failure(...) posting structured error events. |
| src/band/testing/fake_tools.py | Adds send_failure, event-failure simulation, and helper selectors for tests. |
| src/band/testing/init.py | Re-exports events_of_type / reported_failures via lazy exports. |
| src/band/integrations/codex/types.py | Replaces structured error metadata builder with build_agent_failure(...). |
| src/band/integrations/codex/init.py | Updates exports to build_agent_failure and removes remediation export. |
| src/band/integrations/acp/room_emitter.py | Uses deliver_reply to distinguish delivery vs provider failures. |
| src/band/integrations/acp/client_adapter.py | Adds turn_timeout_s and structured failure reporting (incl. JSON-RPC RequestError shaping). |
| src/band/integrations/a2a/adapter.py | Uses deliver_reply and structured failure reporting; protects finally from clobbering exceptions. |
| src/band/integrations/a2a/gateway/types.py | Extends pending-task failure to optionally carry structured failure metadata. |
| src/band/integrations/a2a/gateway/adapter.py | Adds failure redaction/sanitization and emits structured terminal failures for A2A clients. |
| src/band/adapters/anthropic.py | Reports provider failures as AgentFailure (special-cases APIStatusError). |
| src/band/adapters/claude_sdk.py | Migrates to send_failure and carries API error status/errors as code/detail. |
| src/band/adapters/copilot_sdk.py | Reports session/turn failures via send_failure (including session setup errors). |
| src/band/adapters/codex.py | Migrates to send_failure, adds delivery-failure distinction, dedupes failure reports, and removes structured_errors config. |
| src/band/adapters/crewai.py | Migrates to send_failure (including missing-init and missing-reply paths). |
| src/band/adapters/crewai_flow.py | Uses send_failure with capped room-visible failure text and full detail preservation. |
| src/band/adapters/gemini.py | Migrates to send_failure, preserving ServerError status/message into code/detail. |
| src/band/adapters/google_adk.py | Migrates to send_failure, and ensures runner construction/close is safe. |
| src/band/adapters/langgraph.py | Migrates to send_failure with generic safe room-visible message. |
| src/band/adapters/letta.py | Migrates to send_failure, adds delivery failure separation, and reports timeouts consistently. |
| src/band/adapters/opencode/adapter.py | Migrates to send_failure, using HTTP status code as failure code and adding timeout failure code. |
| src/band/adapters/parlant.py | Migrates to send_failure and reports session/init errors without raising in some paths. |
| src/band/adapters/pydantic_ai.py | Migrates to send_failure and broadens provider-failure reporting while preserving prior benign-case behavior. |
| src/band/adapters/strands.py | Migrates to send_failure and ensures transcript/usage preservation only when agent exists. |
| src/band/adapters/agno.py | Migrates to send_failure with generic safe room-visible message and optional RunStatus-based code. |
| tests/testing/test_fake_tools.py | Adds FakeAgentTools send_failure behavior tests. |
| tests/runtime/test_tools.py | Adds AgentTools REST-boundary send_failure behavior tests. |
| tests/core/test_protocols.py | Adds tests for to_failure_event parity and blank-message fallback. |
| tests/core/test_delivery.py | Adds tests ensuring delivery failures wrap as DeliveryFailedError. |
| tests/integrations/parlant/test_tools.py | Updates mocked tools to include send_failure. |
| tests/integrations/claude_sdk/test_dedup_tools.py | Adds coverage that send_failure forwards through dedup wrapper. |
| tests/integrations/acp/test_client_adapter.py | Updates ACP client tests for AgentFailure, RequestError shaping, and turn timeout behavior. |
| tests/integrations/a2a/test_adapter.py | Updates A2A tests for DeliveryFailedError and structured failure metadata. |
| tests/integrations/a2a/gateway/test_adapter.py | Adds/updates gateway tests for structured failure metadata and credential redaction. |
| tests/framework_configs/adapters.py | Updates Codex adapter config expectations (removes structured_errors usage). |
| tests/adapters/test_strands_adapter.py | Updates assertions to align with failure reporting behavior. |
| tests/adapters/test_pydantic_ai_adapter.py | Adds/updates tests to ensure failures are reported via send_failure. |
| tests/adapters/test_parlant_adapter.py | Updates tests to assert send_failure usage and failure content. |
| tests/adapters/test_letta_adapter.py | Updates tests to use reported_failures and validate timeout failure metadata. |
| tests/adapters/test_google_adk_adapter.py | Updates tests to assert send_failure and covers runner construction failure reporting. |
| tests/adapters/test_gemini_adapter.py | Adds/updates tests for generic failures and ServerError shaping into code/detail. |
| tests/adapters/test_crewai_flow_phase4.py | Adds test ensuring capped room-visible failure message with full detail preserved. |
| tests/adapters/test_crewai_adapter.py | Updates tests to assert send_failure and remove legacy send_event error assertions. |
| tests/adapters/test_codex_adapter.py | Updates tests for structured AgentFailure, timeout code, deduped reporting, and delivery-failure non-reporting. |
| tests/adapters/test_claude_sdk_tool_names.py | Updates mocked tools to include send_failure. |
| tests/adapters/test_claude_sdk_adapter.py | Migrates tests from error events to send_failure assertions and adds new failure cases. |
| tests/adapters/test_anthropic_adapter.py | Adds APIStatusError shaping test and migrates error reporting assertions to send_failure. |
| tests/adapters/opencode/helpers.py | Imports centralized events_of_type helper. |
| tests/adapters/opencode/test_setup.py | Uses centralized events_of_type helper. |
| tests/adapters/opencode/test_lifecycle.py | Uses centralized events_of_type helper. |
| tests/adapters/opencode/test_approvals.py | Uses centralized events_of_type and asserts non-AgentFailure procedural timeouts. |
| tests/adapters/opencode/test_turns.py | Migrates error assertions to reported_failures and adds HTTP-status-to-code coverage. |
| tests/adapters/langgraph/conftest.py | Updates mocked tools to include send_failure. |
| tests/adapters/langgraph/test_lifecycle.py | Migrates to send_failure assertions and adds unreported-construction failure coverage. |
| tests/adapters/copilot_sdk/fakes.py | Adds fake client support for session-creation failure injection. |
| tests/adapters/copilot_sdk/test_turn_failure.py | Migrates error assertions to reported_failures and covers session-creation failure reporting. |
| tests/adapters/agno/test_adapter.py | Updates tests to validate failure metadata content for generic and RunStatus errors. |
Review details
- Files reviewed: 61/62 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+607
to
+611
| if isinstance(failure, dict) and isinstance(failure.get("message"), str): | ||
| failure = { | ||
| **failure, | ||
| "message": _redact_credentials(failure["message"]), | ||
| } |
…ng it Several adapters reported a terminal failure via send_failure and then returned normally, which the runtime treats as a successfully processed message -- losing the retry mechanism for a turn that actually failed. Found via cross-SDK comparison against the TS SDK's equivalent ProviderTurnFailedError fix for the same adapters. - letta: on_message's client-not-initialized guard, its session-prep except, and _handle_message's missing-room-context guard now raise after reporting; _run_turn's DeliveryFailedError/TimeoutError/generic except now re-raise instead of swallowing. - parlant: on_message's app-not-initialized guard and session-init except now raise after reporting. - a2a: on_message's DeliveryFailedError and generic except now re-raise instead of swallowing. - claude_sdk: _on_turn_complete's is_error and missing-reply branches now raise TurnResultAlreadyReported after reporting; on_message's outer except recognizes it and does not report the same failure again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195k7AdghCyZgBPgs4TdxE3
Collaborator
Author
Follow-up: cross-SDK review fixes (99b660a)Comparing this PR against band-sdk-typescript's parallel implementation (band-ai/band-sdk-typescript#178) surfaced that several adapters reported a terminal provider failure via
Net effect: across four adapters, a reported provider failure now also fails the turn, so the platform's retry mechanism actually engages instead of silently losing the message. 🤖 Generated with Claude Code |
Collaborator
Author
|
Filed INT-1388 for the "ACP client's per-room timeout isolation" item already called out in this PR's Out of scope section. Confirmed 🤖 Generated with Claude Code |
Consolidates repeated patterns introduced by the AgentFailure migration: shared GENERIC_PROVIDER_FAILURE_MESSAGE constant instead of retyping it per adapter, small _to_agent_failure helpers for anthropic/gemini's isinstance-dispatch branches, a deduped _reraise_delivery_cause helper in codex.py, and dropped a try/except in agno.py around code that can't raise. Also parallelizes independent stop()/send_failure() calls in the ACP client adapter's error paths. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDnFqKrWa9q6B6TJprQeTN
…failure Extends 99b660a's fix to the call sites the code review's follow-up scan found still swallowing a reported failure: ACP client on_message, OpenCode's HTTP/generic exception handlers, A2A's terminal-task-state delivery, CrewAI/ Pydantic AI/Strands/Letta's missing-reply branches, and Codex's turn-timeout, transport/closed, and turn/completed(status=failed) paths -- the latter two were not in the review's finding list but share the exact same bug, found while fixing the turn-timeout sibling in the same function. Promotes TurnResultAlreadyReported to band.core.protocols as the one shared mechanism every adapter uses to avoid double-reporting a failure an inner handler already sent, replacing Codex's adapter-local failure_reported bool for its two now-migrated call sites. Also: - codex/types.py: str()-coerce codexErrorInfo.type before it reaches AgentFailure's strict `code: str | None`, matching every sibling adapter. - a2a/gateway/adapter.py: redact credentials from a relayed peer failure's entire dict (detail/code included), not just its message field. - copilot_sdk.py: wrap the final unguarded send_message in deliver_reply so a delivery failure is reported instead of propagating unhandled. - parlant.py: route the polling-fallback reply through deliver_reply instead of a local log-and-continue try/except (pre-existing, out of this PR's diff, fixed anyway as a small out-of-scope extra). crewai_flow.py's record_failed call sites are deliberately left as return- after-report: that adapter reconstructs state from durable task events each turn rather than a synchronous retry model, so raising there would fight its own design rather than fix a bug. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvve8nDhUac7mf86hHaHp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adopts band-sdk-core's shared
AgentFailureshape (provider/code/message/detail)across band-sdk-python adapters, replacing the free-text
"error"event andits ad hoc per-adapter metadata keys.
Changes
band-sdk-coreto 2.3.0send_failure/to_failure_eventonAgentToolsProtocol,AgentTools,FakeAgentToolsDeliveryFailedError/deliver_replyfor Band-delivery vs. provider-failure separationturn_timeout_s, default 300s) around_runtime.promptPendingA2ATask.fail, credential redaction on both the adapter's own exception path and the peer-forwarded-failure relay pathFAILURE_CODE_TIMEOUTconstant;MessageType.ERRORused instead of a raw"error"literalto_failure_eventblank-content check useshas_visible_content()reported_failures()/events_of_type()test helpers inband.testingfinallyrecord_failedcaps the room-visible message at 500 chars, preserves the full message inAgentFailure.detailOut of scope
band_respond_contact_requesttool handler's own error event (a platform-tool failure, not a provider failure)cancel_turn/stop_reasonpropagationTest plan
uv run ruff check .uv run ruff format --check .uv run pyrefly checkuv run pytest tests/ --ignore=tests/integration/ --ignore=tests/e2e/— 5620 passed, 146 skipped, 0 faileduv run pytest --markdown-docs $(git ls-files '*.md' ':!:examples/*')Follow-up: cross-SDK review fixes (99b660a)
Fixed in 99b660a, found against band-sdk-typescript#178's parallel implementation: several adapters reported a terminal provider failure via
send_failurebut then returned/fell through normally instead of failing the turn.a2a/adapter.pyDeliveryFailedErrorexcepte.causea2a/adapter.pygenericexcept Exceptionsend_failure+ return →send_failure+raiseclaude_sdk.py_on_turn_complete(2 branches)send_failure+ return → raises newTurnResultAlreadyReportedclaude_sdk.pyon_messageexcept TurnResultAlreadyReported: raiseclauseletta.pyclient-not-initialized guardsend_failure+ return → synthesizesRuntimeError, reports, raisesletta.pysession-prepexcept Exceptionsend_failure+ return →send_failure+ bareraiseletta.py_handle_messagemissing-room-contextsend_failure+ return → synthesizesRuntimeError, reports, raisesletta.py_run_turnDeliveryFailedErrorexcepte.cause(still nosend_failure, correctly)letta.py_run_turntimeout / generic exceptsend_failurethen fall through →send_failurethenraiseparlant.pyapp-not-initialized guardsend_failure+ return → synthesizesRuntimeError, reports, raisesparlant.pysession-initexcept Exceptionsend_failure+ return →send_failure+raiseNet effect: across four adapters, a reported provider failure now also fails the turn, so the platform's retry mechanism actually engages instead of silently losing the message.
TurnResultAlreadyReportedprevents this from causing double-reporting inclaude_sdk. Band-side delivery failures remain correctly un-attributed to the provider but are now also retryable.🤖 Generated with Claude Code
https://claude.ai/code/session_0195k7AdghCyZgBPgs4TdxE3