Skip to content

feat: AssetsNN aliases, format_category shim, MCP adoption hooks — unblock salesagent#219

Merged
bokelley merged 3 commits intomainfrom
bokelley/sdk-adoption-plan
Apr 20, 2026
Merged

feat: AssetsNN aliases, format_category shim, MCP adoption hooks — unblock salesagent#219
bokelley merged 3 commits intomainfrom
bokelley/sdk-adoption-plan

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

Follow-up to #218 closing the remaining gaps from downstream (salesagent) feedback ahead of the 4.0 stable cut.

Two commits, independent concerns, split for review:

  • f5c6b18c feat(types)!: AssetsNN semantic aliases + format_category shim + downstream smoke — type surface stability.
  • 2203c1d1 feat(server): context_factory, tenant_id, DISCOVERY_TOOLS — unblock MCP adoption — MCP server hooks.

Types stability (commit 1)

  • AssetsNN semantic aliases. Assets81Assets106 pinned to <Type>FormatAsset / <Type>FormatGroupAsset (VideoFormatAsset, HtmlFormatAsset, RepeatableAssetGroup, etc.). The Format prefix disambiguates format-slot types from the separate asset-content types (VideoAsset, HtmlAsset etc. in adcp.types).
  • tests/test_asset_aliases_stable.py pins each alias to its expected asset_type discriminator default. When datamodel-codegen renumbers between releases, this test fails and points at the specific alias that drifted — the failure mode salesagent hit with Assets5/14 becoming Assets57/149.
  • Deep-submodule format_category shimsys.modules registration so from adcp.types.generated_poc.enums.format_category import FormatCategory raises ImportError with the migration pointer instead of ModuleNotFoundError.
  • Downstream-imports CI smoke — builds the wheel, installs it fresh, imports ~35 representative public-API symbols. Any ImportError regression fails CI before tagging.
  • MIGRATION_v3_to_v4.md appended — full AssetsNN → semantic alias table and the deep-submodule shim doc.

MCP adoption hooks (commit 2)

  • ToolContext.tenant_id: str | None — first-class field for multi-tenant agents. No more smuggling tenant through metadata: dict.
  • create_mcp_server(context_factory=...) — zero-arg callable invoked per tool call; returns a ToolContext (or subclass) populated from whatever the HTTP middleware stashed. ContextVars are the recommended carrier.
  • DISCOVERY_TOOLS frozenset — spec-mandated auth-optional set (get_adcp_capabilities today). Downstream middleware imports this to stay aligned with the spec without maintaining their own list.
  • tests/test_mcp_middleware_composition.py — end-to-end integration test proving Starlette middleware wraps mcp.streamable_http_app(), rejects unauthenticated non-discovery calls with 401, populates ContextVars for authenticated calls, and the handler receives a typed ToolContext with caller_identity + tenant_id.
  • examples/mcp_with_auth_middleware.py — runnable recipe.
  • docs/handler-authoring.md — _impl pattern + idempotency + error classification + multi-tenant typing guidance.

Out of scope (Phase 2, post-stable)

A2A adoption requires pluggable TaskStore, push-notification config persistence, and per-skill middleware hooks — all tracked separately. Downstream keeps their custom A2A server for now; the MCP side is what's production-ready after this PR.

Test plan

  • pytest tests/ --ignore=tests/conformance --ignore=tests/integration — 1491 passed, 15 skipped
  • ruff check src/ tests/ examples/
  • mypy src/adcp/ — 0 errors
  • Manual import sanity for every new symbol on the top-level surface
  • Middleware-composition integration test exercises 401 rejection, discovery bypass, and auth-populated ToolContext round-trip

🤖 Generated with Claude Code

bokelley and others added 3 commits April 20, 2026 01:32
…stream smoke

Follow-up to #218 closing the remaining gaps salesagent flagged around
the 4.0 cut.

AssetsNN renumbering protection
- src/adcp/types/aliases.py: semantic aliases for every Assets* class in
  generated_poc/core/format.py. Individual slots are named
  <Type>FormatAsset (ImageFormatAsset, VideoFormatAsset, ...) — the
  Format prefix disambiguates from the separate asset-content types
  (VideoAsset, HtmlAsset, etc. in adcp.types) which describe payload
  metadata rather than format slot definitions. Repeatable group wrappers
  get <Type>FormatGroupAsset; Assets94 → RepeatableAssetGroup.
- tests/test_asset_aliases_stable.py: pins each alias to its expected
  asset_type discriminator default. When datamodel-codegen renumbers
  AssetsNN between releases, this test fails and tells downstream
  exactly which alias drifted — the failure mode salesagent hit with
  Assets5/Assets14 becoming Assets57/Assets149.

format_category deep-submodule shim
- src/adcp/types/__init__.py: registers a sys.modules shim for the deep
  import path `adcp.types.generated_poc.enums.format_category` so it
  raises an ImportError with the migration pointer instead of the bare
  ModuleNotFoundError that bit downstream. The top-level `from adcp
  import FormatCategory` shim was already in #218; this covers the
  older submodule path some downstreams still use.

Downstream-imports CI smoke
- .github/workflows/ci.yml: builds the wheel, installs it fresh, and
  imports ~35 representative public-API symbols (plus the shim-covered
  removed types). Any ImportError regression fails CI before tagging.
  Proxy for real downstream import sites.

MIGRATION_v3_to_v4.md appended
- Full AssetsNN → semantic alias table with asset_type column
- Deep-submodule shim explained

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…CP adoption

Downstream (salesagent) flagged three hooks missing from adcp.server
that forced them to keep ~600 LOC of wrapper/middleware around the
SDK's create_mcp_server() path. This ships the hooks so they can drop
the wrappers without losing production behavior.

- ToolContext.tenant_id: first-class field, typed. Multi-tenant agents
  no longer have to smuggle tenant through the metadata dict.
- create_mcp_server(context_factory=...): zero-arg callable invoked per
  tool call that returns a ToolContext (or subclass) populated from
  whatever the HTTP middleware stashed. ContextVars are the recommended
  carrier since they compose cleanly with async tasks.
- DISCOVERY_TOOLS frozenset: the spec-mandated auth-optional set
  (get_adcp_capabilities today). Downstream imports it to keep their
  auth middleware aligned with the spec; extending is `| {"..."}` away.

tests/test_mcp_middleware_composition.py is a full integration test:
Starlette middleware wraps mcp.streamable_http_app(), rejects
unauthenticated non-discovery calls with 401, populates ContextVars
for authenticated calls, and the handler receives a typed ToolContext
with caller_identity + tenant_id. examples/mcp_with_auth_middleware.py
is the runnable recipe.

docs/handler-authoring.md captures the _impl pattern + idempotency +
error-classification + multi-tenant typing guidance — the things
downstream figured out the hard way, now documented so the next
adopter pays less tax.

Out of scope here (tracked in .context/sdk-adoption-roadmap.md Phase 2):
A2A pluggable TaskStore, push-notification config persistence, per-skill
audit middleware. Those are the last blockers for full A2A adoption.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…A context_factory, ContextVar safety

Addresses reviewer feedback on PR #219 from parallel code/DX/architecture/
security reviews. Must-fix + should-fix bundled; larger refactors tracked
as follow-up issues.

SECURITY — tenant-scoped idempotency (HIGH)
- src/adcp/server/idempotency/store.py: _extract_principal_id →
  _extract_scope_key. Cache keys now compose tenant_id + caller_identity
  so downstream deployments whose principal IDs are only unique *within*
  a tenant (Okta group-scoped, SCIM per-tenant, seller-internal employee
  IDs) can't leak cached responses across tenants. Backends see an opaque
  scope string.
- src/adcp/server/idempotency/backends.py: principal_id → scope_key
  across MemoryBackend + PgBackend scaffold + schema sketch.
- src/adcp/server/base.py: ToolContext.tenant_id docstring drops the
  unenforceable "principals unique within tenant" invariant and states
  the (tenant_id, caller_identity) cache-key composition it actually
  gets.
- tests/test_server_idempotency.py: locks cross-tenant isolation + the
  positive-case tenant-scope hit with two new tests.

CORRECTNESS — A2A context_factory
- src/adcp/server/a2a_server.py: ADCPAgentExecutor + create_a2a_server
  accept the same context_factory the MCP side takes. A2A preserves its
  ServerCallContext.user → caller_identity fallback when the factory
  leaves caller_identity unset, so adopters who only wire the factory
  on MCP don't silently break A2A auth.
- src/adcp/server/serve.py: serve() threads context_factory into both
  _serve_mcp and _serve_a2a so a single factory populates tenant /
  adapter fields on both transports.

API — RequestMetadata instead of zero-args factory
- ContextFactory signature changed from Callable[[], ToolContext] to
  Callable[[RequestMetadata], ToolContext]. RequestMetadata is a frozen
  dataclass owned by the SDK carrying tool_name, transport, request_id.
- isinstance() guard at every factory callsite catches factories that
  accidentally return a dict before the handler explodes with a deep
  AttributeError.

TEST QUALITY — asset aliases stability contract
- tests/test_asset_aliases_stable.py: _literal_value() reads each
  discriminator field's annotation (Literal[...]) rather than
  FieldInfo.default, which returns PydanticUndefined for several
  generated fields and made the equality check vacuously pass.

TEST INFRA — asgi-lifespan replaces hand-rolled helper
- pyproject.toml: asgi-lifespan added to dev deps.
- tests/test_mcp_middleware_composition.py: drops the 40-line
  _StarletteLifespan class in favor of asgi_lifespan.LifespanManager,
  which surfaces startup exceptions instead of hanging.

SECURITY — example file hardening
- examples/mcp_with_auth_middleware.py:
  - ContextVar.set() now paired with .reset() in a finally block —
    removes the cross-request principal-bleed footgun the test avoided
    but the example taught.
  - Token comparison switched to hmac.compare_digest against SHA-256
    hashes — teaches the constant-time pattern downstream copies to
    production.

DX — docs, error messages, discovery-tool validation
- ImportError messages on removed types now include a
  MIGRATION_v3_to_v4.md#anchor fragment so readers land on the specific
  section. _REMOVED_IN_V4 restructured to (hint, anchor).
- src/adcp/server/mcp_tools.py: validate_discovery_set() helper asserts
  every tool in an auth-optional set resolves to a known tool with
  readOnlyHint=True. Unioning create_media_buy into DISCOVERY_TOOLS
  fails loudly at startup instead of silently unauthenticating writes.
- src/adcp/types/aliases.py: every <Type>FormatAsset docstring carries
  the "distinct from <Type>Asset in adcp.types" one-liner.
- docs/handler-authoring.md: new "15-minute decision tree" at the top;
  Multi-tenant typing section drops temporal "4.0" phrasing and states
  the tenant_id cache-scoping requirement concretely.

CORRECTNESS — format_category shim via real file
- src/adcp/types/generated_poc/enums/format_category.py: new file;
  raises ImportError with the migration pointer at module body
  evaluation. No sys.modules dance — Python's import system picks it up
  natively, so the shim survives any import order.
- scripts/post_generate_fixes.py: new restore_format_category_deprecation_shim()
  rewrites the shim file after codegen wipes generated_poc/.

Deferred as follow-up issues: TypeVar-bound ToolContext subclasses;
renaming <Type>Asset content types to avoid collision with <Type>FormatAsset
slots; advertised-tools gate to stop not_supported defaults from polluting
tools/list; documenting / locking the tools/list pre-auth posture.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit ed7d30a into main Apr 20, 2026
10 checks passed
bokelley added a commit that referenced this pull request Apr 20, 2026
…ses (closes #223)

Roadmap PR-Q + expert-review followup from #219. Multi-tenant agents
routinely subclass ToolContext to carry typed tenant/adapter/testing
fields the base doesn't name — before this PR those fields required
casts at every handler method. Now ADCPHandler is Generic[TContext]
bound to ToolContext; downstream writes
``class MyAgent(ADCPHandler[MyContext])`` and every handler method
signature propagates the subclass type.

API
- New TContext = TypeVar("TContext", bound="ToolContext") exported
  from adcp.server. Docstring at the declaration site explains the
  pattern.
- ADCPHandler now inherits Generic[TContext]. All 57 method signatures
  in base.py rewritten to take context: TContext | None.
- Protocol handlers (BrandHandler, ComplianceHandler,
  ContentStandardsHandler, GovernanceHandler,
  SponsoredIntelligenceHandler, TmpHandler) propagate TContext via
  class X(ADCPHandler[TContext], Generic[TContext]) so downstream
  can write class MyBrand(BrandHandler[MyContext]).
- Internal SDK annotations (mcp_tools.py, serve.py, a2a_server.py,
  builder.py) use ADCPHandler[Any] where the SDK doesn't care about
  the TContext — the decorator-builder path and the transport
  executors don't thread a specific subclass.

Backward compat
- class MyAgent(ADCPHandler) without a TypeVar argument still works
  at runtime. Existing subclasses keep working without edits.
- No runtime behavior change. The TypeVar is purely type-system
  narrowing; handler dispatch paths are unchanged.

Tests — tests/test_handler_typevar.py (9 new, 1544 total)
- Unparameterised subclass still works (backward compat).
- Parameterised ADCPHandler[MyContext] constructs cleanly.
- Protocol handlers propagate the TypeVar (BrandHandler[MyContext]).
- Handler methods receive the subclass at dispatch time — the runtime
  isinstance(context, MyContext) is true.
- BrandHandler[MyContext] propagation tested end-to-end.
- TContext.__bound__ is ToolContext.
- A2A executor dispatches a typed handler without issue.
- Method signature structure preserved (self, params, context positions).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant