fix(decisioning): handler shims for every non-sales wire tool#337
Merged
fix(decisioning): handler shims for every non-sales wire tool#337
Conversation
The Emma DX smoke test (AudioStack creative-generative agent) caught a load-bearing gap: the breadth-sprint shipped 10 Protocol classes + REQUIRED_METHODS coverage at the static layer, but ``PlatformHandler`` only had 9 sales-* shims. Every non-sales wire tool (``build_creative``, ``get_signals``, ``check_governance``, ``get_brand_identity``, ``list_content_standards``, ``create_property_list``, ``create_collection_list``, ...) was 404 at the wire even though capabilities advertised the slug, ``validate_platform`` reported green, and the wire-tool registry at ``mcp_tools.py`` already mapped each tool name to its Request type. A creative-generative adopter wrapping AudioStack (Emma-style real-world test) could implement the Protocol shape correctly, ``serve()`` started, ``tools/list`` returned the advertised set — but ``tools/call name="build_creative"`` got ``Unknown tool: build_creative``. Silent buyer-facing failure with the framework reporting green at boot. Fix: 31 new shims following the existing sales-* template, one per wire tool the breadth-sprint Protocols cover. Per-Protocol-family advertised tool sets, kept as separate frozensets for readability and future selective filtering. The ``advertised_tools`` ClassVar is the union of all 9 sets (40 tools total). New runtime gate: ``_OPTIONAL_PLATFORM_METHODS`` + ``_require_platform_method`` helper. For methods marked optional on the per-specialism Protocol (``preview_creative`` on CreativeBuilderPlatform, ``get_media_buy_artifacts`` / ``get_creative_features`` on ContentStandardsPlatform), the shim pre-checks ``hasattr(platform, method_name)`` and surfaces ``AdcpError(code='UNSUPPORTED_FEATURE')`` to buyers when the adopter chose not to wire the method. Without this, AttributeError would get wrapped to ``INTERNAL_ERROR`` — adopter contract violation, not buyer-fixable. Required methods are caught at server boot by ``validate_platform``; the optional set complements that gate at runtime. Account-resolution: every new shim uses ``getattr(params, "account", None)`` — most non-sales tools don't carry ``account`` on the wire (catalog reads, governance plans, brand-rights queries). The ``AccountStore`` impl handles the no-ref case per its resolution mode (``'derived'`` tolerates None; ``'implicit'`` raises ``AUTH_INVALID``). Arg-projection: ``sync_audiences`` arg-projects ``audiences=params.audiences`` to match the JS-side adopter ergonomic (the AudiencePlatform method signature is ``sync_audiences(audiences, ctx)`` per PR #332). Test coverage in ``tests/test_decisioning_handler_shims.py`` (42 new tests): * ``advertised_tools`` covers every spec wire tool (drift guard against accidentally shipping unrouted advertised tools). * Parametrized: every advertised non-sales tool has a shim method on PlatformHandler. * End-to-end shim → ``_invoke_platform_method`` → platform method for one tool per Protocol family. * ``sync_audiences`` arg-projection regression test. * ``UNSUPPORTED_FEATURE`` surface for optional methods. * AudioStack DX regression test — pinned by name so the Emma fix doesn't silently regress. 2308 tests pass (up from 2266). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
P0:
- F12 auto-emit on 12 webhook-eligible shims (activate_signal,
get_signals, get_creative_delivery, sync_audiences, get_brand_identity,
get_rights, acquire_rights, all 5 property_list ops). Mirrors the
sales-* shim pattern. Fires only when (a) request schema allows
push_notification_config and the buyer registered a URL, and
(b) method_name is in SPEC_WEBHOOK_TASK_TYPES.
- build_creative bare-manifest / list / envelope projection helper
(_project_build_creative) — JS-side projectBuildCreativeReturn parity.
The CreativeBuilderPlatform.build_creative Protocol declares 3 return
arms (single CreativeManifest, list, fully-shaped success envelope);
the wire envelope has only 2 success arms ({creative_manifest} vs
{creative_manifests}). Bare-manifest/list returns now project to wire
shape instead of failing oneOf validation.
- sync_audiences list arm projection (_project_sync_audiences) — wire
envelope is {audiences: [...]} but Protocol allows list-of-rows
ergonomic. Mirrors JS-side wrapping.
- build_creative gated via _require_platform_method so a sales-only
adopter routing here surfaces UNSUPPORTED_FEATURE instead of leaking
AttributeError -> INTERNAL_ERROR.
P1:
- update_rights shim (BrandRightsPlatform mutation: extend term, change
scope, revoke). Added to _BRAND_RIGHTS_ADVERTISED_TOOLS.
- acquire_rights docstring corrected from 3-arm to 4-arm
(acquired/pending/rejected/error).
- Governance/brand-rights/content-standards shims switched from
getattr(params, "account", None) to explicit None for the schemas
that declare additionalProperties:false on the request type — the
account field is forbidden, getattr was a foot-gun for future drift.
get_media_buy_artifacts and get_creative_features keep getattr since
their schemas DO carry account.
Tests:
- _project_build_creative and _project_sync_audiences arm coverage.
- F12 auto-emit fires on get_signals, acquire_rights,
get_creative_delivery, sync_audiences (with projected envelope).
- update_rights routes through the shim.
- update_rights does NOT auto-emit (not in SPEC_WEBHOOK_TASK_TYPES).
- property_list ops do NOT auto-emit (wire schema forbids
push_notification_config); regression-pin so a future schema change
surfaces here.
- build_creative UNSUPPORTED_FEATURE when platform doesn't implement.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
bokelley
added a commit
that referenced
this pull request
May 1, 2026
…irect P0s) (#338) * fix(decisioning): wire-path dispatch + F12 silent no-op (Emma sales-direct P0s) PR #337 merged 31 handler shims that closed the AudioStack 404 issue but Emma's sales-direct backend test (verdict 2/10) surfaced three P0s downstream of the shim layer: every MCP ``tools/call`` 500'd with ``'dict' object has no attribute 'account'``, and buyer-registered push_notification_config.urls were silently dropped when the adopter didn't wire ``webhook_sender``. Unit tests passed because they bypass ``create_tool_caller`` and call shim methods directly. handler.py — moved every ``adcp.types`` Request/Response import out of ``if TYPE_CHECKING:`` so ``typing.get_type_hints()`` resolves the forward refs at runtime. Without this, the dispatcher's typed-params resolver raises ``NameError``, falls back to dict dispatch, and the shim's ``params.account`` access crashes. webhook_emit.py — warn when ``sender=None`` but the buyer registered ``push_notification_config.url``. Adopters who skip ``webhook_sender`` in ``serve()`` now see a WARNING citing the buyer's URL on first call instead of having buyers complain that their notifications never arrive. mcp_tools.py — bumped ``_resolve_params_pydantic_model`` fallback log from DEBUG to WARNING with explicit remediation guidance (import the model at module scope vs. declare ``params: dict``). Silent dispatch fallback is what hid the wire-dispatch break for two PRs. tests/test_decisioning_wire_dispatch.py (new, 29 tests) — pins: - ``typing.get_type_hints`` resolves on every shim - ``_resolve_params_pydantic_model`` returns the typed Pydantic class - ``create_tool_caller`` round-trips a wire-shape dict through the full dispatch path without the params.account crash Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(decisioning): expert-review P1s on PR #338 wire-dispatch fix Three reviewers (code-reviewer, python-expert, adtech-product-expert) converged on five P1/P2 fixes before merge. Wire-dispatch test: - Parametrize over ``PlatformHandler.advertised_tools`` so any new shim auto-extends regression coverage. Was 14 hardcoded tools (28 cases); now 40 (80 cases). - Pin the typed-dispatch contract end-to-end: assert the platform method actually receives a Pydantic ``GetProductsRequest`` / ``BuildCreativeRequest``, not a raw dict. - Drop hardcoded class-name assertions in favor of ``issubclass(resolved, BaseModel)``. Resolver warning (mcp_tools.py): - Surface the failing class name from ``NameError.name`` directly so adopters don't have to parse the bound-method repr. - Trim historical reference per CLAUDE.md. - Forward-compat note for PEP 749 (3.14 lazy annotations). F12 webhook silent-drop (webhook_emit.py): - Move the ``sender=None`` warning ABOVE the full URL/token extraction. A weird ``params`` shape that makes ``_extract_push_notification_url_and_token`` raise mid-traversal would otherwise re-introduce the silent-drop the previous fix was supposed to eliminate. Cheap pre-check on ``getattr(params, "push_notification_config", None)`` decides whether to warn; URL extraction is best-effort for log context. Test count: 2832 passed (was 2777 — +55 from wider parametrize coverage). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
The Emma AudioStack DX smoke test caught a load-bearing gap. The breadth-sprint (PRs #332–#335) shipped 10 Protocol classes + REQUIRED_METHODS coverage at the static layer, but `PlatformHandler` only had 9 sales-* shims. Every non-sales wire tool (`build_creative`, `get_signals`, `check_governance`, `get_brand_identity`, `list_content_standards`, `create_property_list`, `create_collection_list`, ...) was 404 at the wire even though:
This PR closes the gap: 31 new handler shims, one per wire tool the breadth-sprint Protocols cover.
Wins
UNSUPPORTED_FEATUREruntime gate for optional Protocol methods (`preview_creative`, `get_media_buy_artifacts`, `get_creative_features`). Buyers calling an unimplemented optional now get a structured wire error instead of `INTERNAL_ERROR`.advertised_toolsdrift guard — the test `test_advertised_tools_covers_every_specialism_wire_tool` enforces every spec wire tool we ship has both a shim method AND an advertised entry. Adding a new specialism Protocol without wiring the shim will fail this test.Test plan
Follow-up
Re-run the Emma AudioStack test after this lands to confirm the friction report verdict moves from 4/10 ("partial fail") toward 8+/10. If the DX is now crazy-easy for creative-generative, the same pattern should hold for the other Protocol families.
Held release pile (now 11 PRs queued for 4.4.0)
🤖 Generated with Claude Code