Repository navigation
feat(decisioning): let a raised AdcpError reject a request with every error (#1780) - #1810
Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean, additive multi-error support on AdcpError.
No blocking or Medium findings. The reviewer confirmed:
- Entries are normalized and sanitized consistently with
to_wire(). - Malformed entries fail-closed at construction.
- Single-error output is unchanged (backward compatible).
- The
details['errors']lift is well-guarded. - Correct
feat(decisioning)semver signal on an additive-only public surface;errors[]is an existing wire array, so no version-translation pathway is required. - Local mypy deferred to CI per the PR body.
Decision-table walkthrough:
- Row 1 (critical/high): no findings — skip.
- Row 2 (gated paths):
gated_pathsis false — skip. - Row 3 (deleted high-risk): all high-risk files are
(modified), none deleted — skip. - Row 4 (medium data-loss/schema/infra): no medium findings — skip.
- Row 5 (high_risk modified + medium): high_risk is true but there are zero medium findings — skip.
- Row 6 (sticky escalate): no prior decision — skip.
- Row 7 (no-auto-approve team): no team match — skip.
- Row 8 (≥3 medium): zero medium findings — skip.
- Row 9: approve.
The high_risk flag is heuristic; all matches are (modified) with no medium-or-higher concern, so the modifications are presumed safe per the flag-reading rules. review_decision: REVIEW_REQUIRED does not block because gated_paths is false.
… error (#1780) A seller that validates a whole request could only raise one AdcpError, so the payload errors[] carried one entry even when the caller listed every problem in details["errors"]. AdcpError takes errors=[...] (AdcpError, generated Error or wire mapping entries) and AdcpError.from_errors([...]) builds the raise from the list, the first entry filling adcp_error. build_mcp_error_result, the A2A failed-task builder and translate_error write the whole list to errors[]; adcp_error is unchanged. A details["errors"] list whose entries carry a string code and message is lifted into errors[] as well. An entry without recovery takes the raised error's; entry details are sanitized like to_wire(). A single-error raise still mirrors adcp_error into errors[] (#1558). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
64c1ae0 to
50248bb
Compare
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — additive multi-error support on AdcpError, no blocking or medium findings.
What I checked:
- Semver signal is correct:
feat:for additive public-API growth (errors=kwarg,from_errors(),errors_to_wire()), no removed/renamed exports, no required→optional flips, no enum member removal — non-breaking, so a minor cut is appropriate. - Wire-shape fidelity preserved: envelope
adcp_errorvs payloaderrors[]split maintained; single-error mirror unchanged on both MCP and A2A;_lift_detail_errorsis shape-guarded and only touches the documented slot. - Fail-closed construction: malformed/non-mapping entries raise ValueError/TypeError at construction; per-entry sanitization via
sanitize_error_details. - DR-0032/0033 pathway:
errors[]enrichment is a backward-compatible response projection (not a mutating request field), so no version-translation pathway is owed. - Tests pin the behavior: A2A projection assertions (L538-546), end-to-end executor test (L570-578), and
test_from_errors_envelope_is_errors_zero(L489-498) lockingadcp_error == errors[0].
High-risk flag: true, but all three matches are (modified) files with no medium-or-higher findings — presumed safe per the modified-file rule. No deletions.
Gate posture: gated_paths is false; no no-auto-approve team match. review_decision is REVIEW_REQUIRED, but the deprecated protected-branch row no longer forces comment, and gated_paths=false means row 2 does not fire. Subsequent pass on a prior approve with no new findings. Falls through to row 9.
…#1819) The conventional-commit policy stripped a trailing "(#N)" from every message, including PR titles. #1810's title ended in "(#1780)", passed as if that were the PR number, and integrated as "... (#1780) (#1810)", which the main check rejects. Main CI failed at the 9.5.0 release commit, so Release publish refused to publish 9.5.0. PR titles are now checked as written, since the squash appends the PR number only at integration. 272fced keeps its subject through an exact-commit-and-subject exception, like the historical breaking-footer ones. Every main commit since the policy floor validates. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Problem
A seller that validates a whole
create_media_buy/update_media_buyrequest finds several problems at once, but a decisioning platform can only report them by raisingAdcpError, which carries one code, message and field.build_mcp_error_resultand the A2A failed-task builder wroteerrors: [<that one error>], even when the caller put the whole list indetails["errors"](the slotAdcpError's docstring named for multi-error preflight). The issue's repro printed1 [None]. Sellers had to lift the list in a response enhancer themselves.Change
AdcpError(..., errors=[...]): entries areAdcpErrors, generatedErrormodels, or wire mappings with a stringcodeandmessage. The raised error still fills the envelopeadcp_error, and every entry goes into the payloaderrors[]. An entry withoutrecoverytakes the raised error's. Entrydetailsgo throughsanitize_error_detailsliketo_wire(). A malformed entry raises at construction (TypeError/ValueError).AdcpError.from_errors([...])builds the raise from the list, with the first entry fillingadcp_error, soadcp_error == errors[0]. It returns a baseAdcpErroreven when the first entry is a subclass.AdcpError.errors_to_wire()returns the payload list: every entry, or[to_wire()]when there is no list.details["errors"]slot is lifted. Whenerrors=is not passed and everydetails["errors"]entry is a mapping with a stringcodeandmessage, those entries fillerrors[]. Any other shape (strings,{"path", "msg"}dicts, an empty list) is left alone, as before.adcp_error.detailsis unchanged.build_mcp_error_result, the A2A executor's failed-task DataPart andtranslate_error(..., "a2a")write the whole list. A single-error raise still mirrorsadcp_errorintoerrors[](RaisedAdcpErroronly fillsadcp_error, so storyboards that readerrors[0]fail #1558/fix(server): mirror a raised AdcpError into the payload errors array #1592), so existing output is unchanged.docs/handler-authoring.md.Not covered: an async
TaskHandofffailure still records one error throughregistry.fail(task_id, exc.to_wire()), sotasks/getand the failure webhook reporterrors: [adcp_error]. Carrying the list there means a task-registry storage change, so it's left as a follow-up.Adopter note: this change is additive. A seller whose response enhancer already lifts
details.errorsintoerrors[](as the Scope3 sales agent'sproject_wire_errorsdoes) gets the list from the SDK now. An enhancer that replaceserrorskeeps working; one that appends would duplicate the entries and should be removed.Tests
tests/test_adcp_error_multi.py(28 tests) covers:from_errorswith all three entry types, and the subclass entry;details["errors"]lift, including the shapes it must leave alone;build_mcp_error_result;_register_tool;translate_errorA2A data, and the A2A executor's failed task.Backport of #1804 (SDK 10,
next) to SDK 9: same commit, cherry-picked cleanly. Additive, no breaking change. On this branch the 149 targeted tests andmake lintpass locally;make typecheck-allpasses locally too.Closes #1780
🤖 Generated with Claude Code