Conversation
…o request context (closes #563) PR #560 fixed context-echo on the synchronous AdcpError raise path. Background tasks (TaskHandoff) route through a separate path: registry.complete on success and registry.fail on AdcpError or wrapped INTERNAL_ERROR. Neither echoed the request's context extension, so a buyer polling tasks/get on a deferred task lost their correlation IDs across the handoff boundary. _project_handoff now accepts request_params (the original Pydantic model). Both _fail and the success branch apply inject_context to the wire dict before passing to the registry. Symmetric across: - sync MCP / A2A success path (mcp_tools.py / a2a_server._send_result) - sync AdcpError path (serve.py via build_mcp_error_result) - bg task success (registry.complete) - bg task failure incl. wrapped INTERNAL_ERROR (registry.fail) A new _to_request_dict helper coerces Pydantic to dict for inject_context, with empty-dict fall-through on coercion failure. 4 new tests in test_decisioning_dispatch.py cover success-path echo, AdcpError echo, wrapped-INTERNAL_ERROR echo (RuntimeError → wrap → echo), and no-request-params no-op. 42 dispatch tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nse top level Protocol-expert flagged the original PR put context inside result/error on the registry record. Per schemas/cache/core/tasks_get_response.json context is a top-level sibling of result/error/history. Spec-incorrect shape would have surfaced as result.context / error.context on tasks/get reads instead of top-level context. Refactor: - TaskRecord gains a request_context field; to_dict surfaces it at the top level under context key. - TaskRegistry.issue accepts request_context kwarg (additive, default None — backwards-compatible for adopters who don't thread context). - _project_handoff and _project_workflow_handoff (the second gap reviewer flagged — workflow path also issues tasks) extract context from request_params and pass to issue() once at issue-time. Terminal-state helpers (_fail, registry.complete) no longer touch context. - _to_request_dict renamed to _extract_request_context. model_dump failures now log a warning (was silent before). Tests assert top-level rec[context] (not rec[result][context] or rec[error][context]) per the spec shape. 129 tests across registry/idempotency/dispatch green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Updated based on expert review (code-reviewer + security-reviewer + ad-tech-protocol-expert): Spec-correctness fix (protocol-expert): Original approach injected context inside the result/error dict. Per
Workflow_handoff coverage (reviewer #2): The same context-echo gap existed in Observability fix (security-reviewer): Tests updated to assert top-level Open question I deferred (security-reviewer L1): credential-shaped keys in the buyer's 129 tests across registry/idempotency/dispatch green; lint clean. |
|
Thanks for the detailed update. The spec-correctness refactor ( Ready for human review when you are. Triaged by Claude Code. Session: https://claude.ai/code/session_01WyXp7wGvN7oX9VVcwxt9iR Generated by Claude Code |
…nit) Reviewer flagged that adding the request_context kwarg breaks custom TaskRegistry impls in the wild — Python doesn't silently absorb unknown kwargs at the call site. Adds **_extra: Any to both the Protocol declaration and InMemoryTaskRegistry.issue so the framework can introduce new optional kwargs in future versions without breaking adopters who haven't upgraded their custom impls. Custom impls MUST include **_extra: Any on their issue() signature. Documented the contract in the Protocol docstring; InMemoryTaskRegistry logs unrecognized kwargs at DEBUG so adopters notice they've fallen behind without noisy warnings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed the P3 nit: Documented the forward-compat contract in the Protocol docstring: custom impls MUST include Tests still green. Ready for admin-merge. |
|
The Triaged by Claude Code. Session: https://claude.ai/code/session_013aJ8fiv2FgJedEFoBEuHrV Generated by Claude Code |
Closes #563 (follow-up to #560).
The gap
PR #560 fixed context-echo on the synchronous AdcpError raise path across MCP and A2A. Background tasks (TaskHandoff) route through a separate path that #560 didn't touch: `registry.complete(task_id, result)` on success and `registry.fail(task_id, error.to_wire())` on AdcpError (or wrapped INTERNAL_ERROR). Neither echoed the request's `context` extension, so a buyer polling `tasks/get` on a deferred task lost their correlation IDs across the handoff boundary.
The fix
`_project_handoff(...)` now accepts `request_params: BaseModel | None`. Both terminal-state helpers — `_fail` and the success branch that calls `registry.complete` — apply `inject_context(_to_request_dict(request_params), wire_dict)` before passing to the registry.
The echo is now symmetric across the four terminal-state paths:
New helper
`_to_request_dict(params)` coerces a Pydantic request model (or dict) to a plain dict for `inject_context`. Empty-dict fall-through on coercion failure — the echo is a feature, not a load-bearing invariant; if Pydantic fails to dump, the wire envelope still goes out without crashing.
Tests
4 new in `test_decisioning_dispatch.py`:
42 dispatch tests green; ruff clean.
Test plan
🤖 Generated with Claude Code