You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up from #1547 re-review. The "top-level / pre-handler failure surfaces the AdCP two-layer error envelope on the wire (not a transport-native error)" contract is currently graded — and on REST, implemented — only partially across transports.
State per transport
A2A — covered. on_message_send's outer handler returns a failed Task carrying the two-layer envelope; graded by @T-UC-002-ext-nl-unsupported (A2A-only, correctly, since NL dispatch is an A2A entry point).
MCP — envelope is carried. _translate_to_tool_error (src/core/tool_error_logging.py) runs normalize_to_adcp_error() on any exception and builds the two-layer envelope, so an untyped top-level failure still surfaces AdCPToolError with the envelope. (Message-scrub for untyped exceptions is the separate concern in fix: sanitize untyped-error messages before the buyer-facing wire/webhook #1587.)
REST — gap.src/app.py registers exception handlers for AdCPError, ValueError, RequestValidationError, PermissionError, and ToolError, but there is no@app.exception_handler(Exception) catch-all. An untyped top-level exception (e.g. RuntimeError, a DB/OS error) escaping a REST endpoint gets FastAPI's default bare 500 with no AdCP envelope.
Proposed fix
Add a catch-all @app.exception_handler(Exception) in src/app.py that routes through safe_adcp_error(exc) (leak-safe: untyped → generic SERVICE_UNAVAILABLE envelope, no str(exc)) and returns _envelope_response(...), mirroring the MCP boundary. Framework exceptions with their own handlers (HTTPException, RequestValidationError) are unaffected — a generic Exception handler only catches what no more-specific handler claims.
Add per-transport integration tests grading "untyped top-level failure → two-layer envelope" on REST and MCP (A2A already covered), each asserting the same envelope values in that transport's shape.
Scope note
Split out of #1547 deliberately: #1547's scope is A2A error routing, and a REST catch-all changes every REST endpoint's untyped-500 shape — a cross-transport parity change that deserves its own review. Adjacent to #1587 (which scrubs the untyped-error message); the catch-all here should use the same safe_adcp_error sink, so the two are natural to land together.
Follow-up from #1547 re-review. The "top-level / pre-handler failure surfaces the AdCP two-layer error envelope on the wire (not a transport-native error)" contract is currently graded — and on REST, implemented — only partially across transports.
State per transport
on_message_send's outer handler returns a failed Task carrying the two-layer envelope; graded by@T-UC-002-ext-nl-unsupported(A2A-only, correctly, since NL dispatch is an A2A entry point)._translate_to_tool_error(src/core/tool_error_logging.py) runsnormalize_to_adcp_error()on any exception and builds the two-layer envelope, so an untyped top-level failure still surfacesAdCPToolErrorwith the envelope. (Message-scrub for untyped exceptions is the separate concern in fix: sanitize untyped-error messages before the buyer-facing wire/webhook #1587.)src/app.pyregisters exception handlers forAdCPError,ValueError,RequestValidationError,PermissionError, andToolError, but there is no@app.exception_handler(Exception)catch-all. An untyped top-level exception (e.g.RuntimeError, a DB/OS error) escaping a REST endpoint gets FastAPI's default bare500with no AdCP envelope.Proposed fix
@app.exception_handler(Exception)insrc/app.pythat routes throughsafe_adcp_error(exc)(leak-safe: untyped → genericSERVICE_UNAVAILABLEenvelope, nostr(exc)) and returns_envelope_response(...), mirroring the MCP boundary. Framework exceptions with their own handlers (HTTPException,RequestValidationError) are unaffected — a genericExceptionhandler only catches what no more-specific handler claims.Scope note
Split out of #1547 deliberately: #1547's scope is A2A error routing, and a REST catch-all changes every REST endpoint's untyped-
500shape — a cross-transport parity change that deserves its own review. Adjacent to #1587 (which scrubs the untyped-error message); the catch-all here should use the samesafe_adcp_errorsink, so the two are natural to land together.