@IdempotencyStore.wrap incompatible with arg-projected methods (update_media_buy)

[bokelley]

What

@IdempotencyStore.wrap (src/adcp/server/idempotency/store.py:131-219) wraps the handler with signature (handler_self, params, context=None, *args, **kwargs). But the framework's arg-projector (src/adcp/decisioning/dispatch.py:~1080) splits some tools into per-field kwargs:

# dispatch.py:1085 (arg_projector branch for update_media_buy)
result = await method(**arg_projector, ctx=ctx)
# arg_projector = {"media_buy_id": "...", "patch": {...}}

When update_media_buy is decorated with @_IDEMPOTENCY.wrap, the wrap body never runs — Python raises before:

TypeError: MockSellerPlatform.update_media_buy() missing 1 required positional argument: 'params'

Strictly a calling-convention incompatibility. Dispatch passes media_buy_id=..., patch=..., ctx=... as kwargs. None match the params positional, no default, the projected kwargs land in **kwargs, and Python errors out before _wrapped's body executes. So this isn't a hash collision or cache-key issue — the wrap doesn't get a chance to compute anything.

Repro

salesagent's MockSellerPlatform.update_media_buy decorated with @_IDEMPOTENCY.wrap. Method signature is (self, media_buy_id, patch, ctx) per the framework's arg-projector contract. Calling through MCP storyboard runner against core/ raised the TypeError above on every update_media_buy request.

Reference: same shape as examples/v3_reference_seller/src/platform.py:735 which intentionally does NOT wrap update_media_buy (presumably for the same reason — neither the example nor the docs spell this out as a known incompatibility).

Workaround we shipped

Dropped @_IDEMPOTENCY.wrap from update_media_buy on both core/platforms/mock.py and core/platforms/gam.py. We've lost idempotency on update_media_buy — buyer retries re-execute side effects. Acceptable for the spike; not for prod.

What we'd want

Cleanest fix on the wrap side: make wrap signature-flexible so it works regardless of how the framework calls it. Detect the arg-projector calling pattern (kwargs that aren't ctx/context) and synthesize a params dict from those kwargs before hashing:

async def _wrapped(handler_self, params=None, context=None, *args, **kwargs):
    if params is None and kwargs:
        # Arg-projector calling pattern — synthesize params from non-ctx kwargs
        ctx_kw = kwargs.pop("ctx", None) or kwargs.pop("context", None)
        params = kwargs
        if ctx_kw is not None:
            context = ctx_kw
    # ... existing logic, then dispatch back via the same calling pattern
    return await handler(handler_self, **params, ctx=context)  # for arg-projected
    # OR: return await handler(handler_self, params, context, *args, **kwargs)  # for normal

(Sketch only — needs tightening on which calling pattern was used so the inner handler(...) invocation matches.)

Alternative on dispatch side: detect a wrapped handler and route via method(params=arg_projector_dict, ctx=ctx) instead of unpacking. But dispatch is generic and shouldn't know about @wrap, so the wrap-side fix is cleaner.

Filed by

salesagent kill-nginx/M2 spike — surfaced in core/SDK_FEEDBACK.md round 3 along with #558 (BearerTokenAuth × transport=both).

[bokelley]

Triage

Classification: Bug
Bucket(s): middleware (idempotency)
Status: drafting-pr

What the experts said:

• ad-tech-protocol-expert: approved design — threading __adcp_params__ is the correct fix; naive synthesis from projected kwargs fails because idempotency_key lives on the Pydantic model, not in the projected field subset, and the subset can't be serialized by rfc8785
• code-reviewer: approved after blocker fix — flagged that direct unit-test calls without the sentinel would fall into _prepare(None, ...) and raise a confusing TypeError; fixed by sentinel-absent fall-through; also flagged kwargs = {} fragility (→ kwargs.clear()) and sync executor dead-code path (documented with inline comment)

My take: The params parameter in _wrapped was required-positional, which breaks under arg-projector dispatch that passes no positional args. The fix threads the original wire-shape Pydantic model via a __adcp_params__ sentinel kwarg (only when is_wrapped is True at the dispatch callsite), giving the wrapper the full model for hashing without changing the calling convention for unwrapped methods. 5 regression tests added covering the arg-projected path end-to-end.

Drafting PR: fixes _wrapped signature, threads sentinel from dispatch, adds sentinel-absent fall-through for direct test calls, 5 new tests.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01HopGAcSiNk5SWbLpE9fd6b

---

Generated by Claude Code

[bokelley]

Triage

Classification: Bug
Bucket(s): middleware (label absent — gap noted in run summary)
Status: ready-for-human

What the experts said:

• code-reviewer: params=None sentinel creates false-positive arg-projector detection; idempotency_key is unreachable in the synthesized dict (it lives inside the nested patch field, not top-level); no tests for arg_projector calling convention — 3 blockers.
• security-reviewer: params = dict(kwargs) when kwargs contains Pydantic model values feeds un-serializable data to rfc8785, causing hash failure or permanent false IDEMPOTENCY_CONFLICT on cross-process retries; params=None condition enables idempotency bypass on zero-body requests — 2 blockers.

My take: The author's sketch fixes the TypeError but leaves idempotency silently broken for update_media_buy — even after the call succeeds, _prepare finds no idempotency_key at the top level of the synthesized {"media_buy_id": ..., "patch": <model>} dict because the key is nested inside patch. The architecturally correct fix requires changes to both dispatch.py and store.py: _invoke_platform_method should pass explicit sentinel kwargs (_adcp_arg_projected=True, _idempotency_params=<original_request>) alongside the projected kwargs, and _wrapped should pop and use _idempotency_params for hashing rather than reconstructing params from kwargs. Flagging because the change touches the security-critical per-principal scoping path and the naive approach has five expert-identified blockers across correctness, security, and test coverage.

/cc @bokelley

---

Triaged by Claude Code. Session: https://claude.ai/code/session_019pauPrwFyu6tbAuQHzDcco

---

Generated by Claude Code