fix(idempotency): support @wrap on arg-projected methods (update_media_buy)#564
Closed
fix(idempotency): support @wrap on arg-projected methods (update_media_buy)#564
Conversation
…a_buy) IdempotencyStore.wrap now handles the arg-projector calling convention used by dispatch for update_media_buy and similar methods. Dispatch threads the original wire-shape Pydantic model as __adcp_params__ so the wrapper can hash the canonical RFC 8785 flat request (with idempotency_key at the top level) rather than the projected subset. The sentinel-based ctx extraction avoids the falsy-object bypass identified in security review. Fixes #559 https://claude.ai/code/session_01HopGAcSiNk5SWbLpE9fd6b
…a_buy) Closes #559. Dispatch threads the original wire-shape Pydantic model as __adcp_params__ when calling idempotency-wrapped methods via the arg-projector path, so the wrapper can hash the canonical flat request (RFC 8785 / AdCP #2315) rather than the projected field subset which neither carries idempotency_key nor serializes cleanly. Direct calls without the sentinel (adopter unit tests) now fall through to the handler instead of raising TypeError inside _prepare. https://claude.ai/code/session_01HopGAcSiNk5SWbLpE9fd6b
Contributor
Author
|
Superseded by #567 (merged today as 1abdae6), which closed #559 with the wrap-side resolution architecture rather than the dispatch-side sentinel approach this draft proposed. Per CLAUDE.md's parallel-race guidance for stale claude-triaged draft PRs: closing rather than rebasing, since the two architectures aren't compatible (#567 explicitly chose against threading dispatch→idempotency coupling). Issue #559 already in 4.4.0 changelog via #567. |
Contributor
Author
|
Noted — supersedence by #567 (1abdae6) confirmed. Issue #559 is closed via the wrap-side resolution. No further action on this draft. Triaged by Claude Code. Session: https://claude.ai/code/session_01Rvm9f1JFJLjBfyvVrXVe1A Generated by Claude Code |
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.
Closes #559
Summary
@IdempotencyStore.wrapraisedTypeError: missing required positional argument: 'params'when applied to methods the framework dispatches via the arg-projector path (e.g.update_media_buy). The arg-projector callsmethod(media_buy_id=..., patch=..., ctx=ctx)with no positionalparams— so the wrapper'sparams: Anyparameter was never filled.Root cause: the wrapper had no way to obtain the canonical wire-shape Pydantic model needed for RFC 8785 hashing and
idempotency_keyextraction, since the projected kwargs are a field-subset and cannot be reconstituted into a valid model.Fix approach: dispatch now threads the original
paramsobject as__adcp_params__=paramswhenis_wrapped(method)is True, giving the wrapper the full model without touching the normal call site. The wrapper pops the sentinel, detects the arg-projector convention (params is None and len(field_kwargs) > 0), and uses the canonical model for hashing while forwarding the original projected kwargs to the handler.Edge case fixed by pre-PR review: direct adopter unit-test calls (e.g.
seller.update_media_buy(media_buy_id=..., ctx=ctx)without dispatch or sentinel) previously would have fallen into_prepare(None, ...)and raised a confusingTypeErrorinside_to_dict. Now the wrapper falls through to the handler whenarg_projected=Trueand the sentinel is absent, so wrapped methods are unit-testable without going through dispatch.Changes
src/adcp/server/idempotency/store.py—_MISSINGsentinel, updated_wrappedsignature (params: Any = None), arg-projector detection, sentinel pop, sentinel-absent fall-through,kwargs.clear()for claritysrc/adcp/decisioning/dispatch.py—is_wrapped as _is_idem_wrappedimport; conditional__adcp_params__=paramsinjection on both async and sync executor paths; inline comment on the sync path (forward-compat only, sincewrapalways returns an async def)tests/test_server_idempotency.py— 5 new tests inTestArgProjectedCallingConvention: no-TypeError, cache-hit dedup, payload conflict, no-key fall-through, and direct-call-without-sentinel fall-throughTested
pytest tests/test_server_idempotency.py— 54 passed (5 new regression tests)pytest tests/(full suite) — 3,940 passed, 1 pre-existing failure (TLS network test unrelated to this change)mypy src/adcp/— 0 errorsruff check src/— 0 errorsPre-PR review
TypeErrorin_prepare) resolved by sentinel-absent fall-through;kwargs = {}→kwargs.clear()for clarity; sync executor dead path documented with inline comment. 1 nit surfaced below.idempotency_keyextraction from canonical model is sound.Nit (not blocking):
__adcp_params__is not a reserved kwarg at the Pydantic level — a request model with a field named__adcp_params__would have its field value shadowed at the call site by the sentinel. The double-underscore prefix is a reasonable deterrent; the sentinel name is documented instore.py's module docstring as reserved in the arg-projector calling convention.Session: https://claude.ai/code/session_01HopGAcSiNk5SWbLpE9fd6b
Generated by Claude Code