feat: universal request normalization for AdCP backward compatibility

[KonstantinMirin]

Summary

Adds a universal request normalization layer and real end-to-end transport testing infrastructure. What started as fixing brand_manifest rejection (#1151) evolved into a comprehensive overhaul of how the seller agent handles schema evolution, validates input, and tests across transports.

Closes #1151, #1180. Supersedes #1161.

The Problem

Three problems, one root cause — the AdCP spec evolves faster than implementations can track:

1. MCP hard-rejected deprecated fields (brand_manifest → ValidationError) while A2A and REST silently dropped them. Publishers blocked.
2. FastMCP's TypeAdapter validates before our Pydantic models — a second validation gate we don't control, creating inconsistent behavior across transports.
3. Test infrastructure bypassed real transport paths — call_mcp() called tool functions directly with a mock Context, never exercising middleware, TypeAdapter, or auth. Bugs were invisible.

What This PR Does

1. Universal Request Normalization

Single normalize_request_params() function shared by all three transports. Translates deprecated AdCP fields:

Deprecated	Current	Scope	Translation
brand_manifest	brand	get_products, create_media_buy	Extract domain from URL → {domain: hostname}
campaign_ref	buyer_campaign_ref	create_media_buy only	Rename
account_id	account	All tools	Wrap: "x" → {account_id: "x"}
optimization_goal	optimization_goals	Package-level	Scalar → array
catalog	catalogs	Package-level	Scalar → array
promoted_offerings	catalogs	All tools	v2.5 rename

Integrated via:

• MCP: RequestCompatMiddleware (FastMCP on_call_tool — official API, no monkey-patching)
• A2A: Called in _handle_explicit_skill() dispatcher
• REST: RestCompatMiddleware (Starlette middleware normalizes JSON body)

2. Schema-Aware Deep-Strip Middleware (Production Forward-Compat)

The middleware has a three-stage pipeline, environment-gated:

Stage	Dev Mode	Production Mode
1. Translate deprecated fields	✅ Always	✅ Always
2. Strip unknown top-level fields	❌ Fail loudly (schema drift detection)	✅ Strip with WARNING
3. Deep-strip + retry on TypeAdapter rejection	❌ Fail loudly	✅ Recursive strip → retry

Stage 3: Deep-strip is the key innovation. When TypeAdapter rejects arguments (e.g., buyer sends a future field inside a nested object with additionalProperties: false), the middleware:

1. Gets the tool's full JSON Schema via tool.parameters
2. Recursively walks the value alongside the schema tree
3. At each object level with additionalProperties: false, strips unknown properties
4. Handles anyOf/oneOf unions by scoring variants on declared-property matches
5. Handles allOf by merging properties from all members
6. Retries with cleaned arguments — TypeAdapter accepts, Pydantic models validate

This is schema-independent from AdCP — strips against what our tool actually accepts, not what the external spec says. If stripping didn't change anything (e.g., type mismatch, not unknown fields), the original error propagates — no infinite retry loop.

Architecture principle: TypeAdapter is the gate we bypass. Everything after it (Pydantic model validation, _impl business logic) is intentional validation whose errors must reach the buyer. The middleware never swallows business logic errors.

3. Real End-to-End Transport Testing

Before: call_mcp() called tool functions directly with a mock Context. No middleware, no TypeAdapter, no auth chain. Bugs invisible.

After: Every transport exercises the real pipeline:

Transport	Path	Auth
MCP	Client(mcp) in-memory → middleware chain → TypeAdapter → tool fn	Real: get_http_headers → token → DB lookup
A2A	AdCPRequestHandler.on_message_send() → skill routing → normalization → model_validate → handler	Real: _resolve_a2a_identity (single mock point)
REST	Starlette TestClient → middleware → FastAPI route → Pydantic body	Real: dependency injection auth
impl	Direct _impl() call	Identity object

4. Forward-Compatibility Test Suite (100+ tests)

Comprehensive test coverage for the deep-strip mechanism, derived using systematic three-pass methodology:

test_deep_strip.py (56 tests):

• P1-P8: Postcondition scenarios for each stripping behavior (top-level, nested, arrays, unions, refs, open schemas)
• INV-1: Known fields never removed
• INV-2: Stripping is idempotent
• INV-3: Data preservation — known field VALUES are bit-identical after strip (strings, unicode, ints, floats, bools, None, empty strings)
• Adversarial: mixed additionalProperties in anyOf, allOf handling, oneOf discriminated unions, 10-level nesting

test_forward_compat_acceptance.py (36 tests):

• MCP pipeline: 9 payload shapes accepted in production (current spec, future fields, v2.5 legacy, multi-level extras)
• Dev mode: 3 tests verify unknown fields rejected loudly
• Deep-strip retry E2E: nested extras → TypeAdapter rejects → strip → retry → succeeds
• Data preservation E2E: intercept _impl to verify tool receives buyer's data intact after strip
• Error propagation: 5 adversarial tests — business logic errors, AdCPError, RuntimeError all propagate (never retried)
• Middleware adversarial: schema lookup failure → graceful degradation, concurrent calls → no interference, retry error propagates (not stale original)

test_mcp_compat_middleware.py (10 tests): _should_retry distinguishes TypeAdapter from business logic errors

test_mcp_client_dispatch.py (3 tests): Client(mcp) pipeline baseline

5. Review Fixes

• get_media_buys account_id bug: A2A handler bypassed model_validate(), using parameters.get("account_id") which returned None after normalization. Fixed: handler uses model_validate(), GetMediaBuysRequest gains account: AccountReference | None field. MCP/raw wrappers updated to account: dict | None.
• Dead except PermissionError: Removed from A2A skill dispatch — AdCPAuthorizationError (subclass of AdCPError) already caught above.
• tests/harness/ in tox: Added to unit env so harness meta-tests and dispatch tests run in CI.
• Defensive json.dumps: error.details serialization wrapped in try/except to prevent crashes on non-serializable values.
• ToolError branch dropped: _should_retry only catches pydantic.ValidationError with title.startswith("call["). TypeAdapter never raises ToolError.
• Redundant schema alignment test deleted: Tested spec against library (circular — library is generated from spec). Broke whenever spec site updated ahead of library release.

6. Transport-Agnostic Error Assertions

Added assert_rejected(result, field=..., reason=...) helper that checks observable behavior: "was this request rejected because field X had problem Y?" Works across all transports without knowing which layer caught the error.

7. Additional Fixes

• BDD empty-step guard: Extended to catch empty @given/@when steps (previously only @then)
• CreativeAssetFactory: Pydantic schema factory for valid CreativeAsset objects
• ToolError details preservation: Error details serialized as JSON 4th arg in ToolError, parsed back by unwrapper
• MCP tool signature fixes: sync_accounts, list_authorized_properties migrated from req: to individual typed params

Known Limitations

• 5 BDD tests xfailed (refactor: move billing policy and approval mode from identity to tenant configuration #1184): supported_billing and account_approval_mode are stored on identity by test harness but don't exist in DB. Real MCP auth resolves identity from DB → policies missing → billing/approval not enforced. Needs DB migration.
• Nested forward-compat is MCP-only: adcp library types (BrandReference, AccountReference) use extra='forbid' hardcoded. Deep-strip handles this on MCP. A2A/REST rely on production extra='ignore' for top-level fields, but nested library types always reject unknowns. Upstream fix: adcp library could use extra='ignore' on these types.

Test Results

unit:        4222 passed, 0 failed
harness:      109 passed, 0 failed (deep-strip, forward-compat, dispatch)
admin:          10 passed, 0 failed
e2e:            93 passed, 0 failed
integration: 1848 passed, 0 failed
bdd:         1068 passed, 0 failed, 5 xfailed (#1184)

Follow-up Issues

• fix: empty BDD Given step bodies from #1170 need real implementations #1181: Empty BDD Given step bodies from feat: Account management, adcp 3.10 migration, and BDD test infrastructure #1170
• fix: MCP tool signature gaps exposed by real TypeAdapter validation #1182: MCP tool signature gaps (remaining tools)
• feat: agent-facing tool descriptions, parameter schemas, and error suggestions #1183: Agent-facing tool descriptions (0/150 params have descriptions)
• refactor: move billing policy and approval mode from identity to tenant configuration #1184: Move billing/approval policy from identity to tenant DB config

[KonstantinMirin]

Note on guard strengthening: While rebasing onto main, the BDD empty-step guard (which this PR extends to cover @given/@when steps, not just @then) caught two empty Given step bodies that slipped in from #1170:

• admin_accounts.py:given_tenant_exists — empty body
• uc002_create_media_buy.py:given_account_not_exists — empty body

These are allowlisted for now. Filed #1181 to fix them separately (different branch/scope).

[ChrisHuie]

think you need to include the path tests/harness/ to the tox.ini test envs for this file to run.

[KonstantinMirin]

Fixed in 4b8b0db — added tests/harness/ to the tox unit env. That also picks up the 83 existing harness meta-tests that weren't being collected. Verified with tox -e unit: 4265 passed.

[ChrisHuie]

Can you add some extra tests for the middleware.

• call_next raises ValidationError in production
• _should_retry returns False in development
• _get_known_params returns correct tool schemas
• non-retry exceptions propagate

[ChrisHuie]

Unscoped account_id normalization could break get_media_buys

Files:

• src/core/request_compat.py:111-115 — the unscoped deletion
• src/a2a_server/adcp_a2a_server.py:1330 — normalization call site
• src/a2a_server/adcp_a2a_server.py:1924 — broken consumer
• src/core/mcp_compat_middleware.py:51 — MCP normalization call site

Example:

A buyer sends:
{"skill": "get_media_buys", "parameters": {"buyer_refs": ["REF-001"], "account_id": "ACC-123"}}

Here is the flow:

1. normalize_request_params("get_media_buys", parameters) runs at adcp_a2a_server.py:1330
2. Lines 111-115: deletes account_id and creates account: {account_id: "ACC-123"}
3. Parameters then become: {"buyer_refs": ["REF-001"], "account": {"account_id": "ACC-123"}}
4. Handler at line 1924: parameters.get("account_id") then returns None
5. Then _get_media_buys_impl skips account filtering (the if req.account_id is not None check at the impl level doesn't fire)
6. Buyer receives media buys from ALL accounts instead of just ACC-123

Only get_media_buys seems to be an issue. Other handlers either don't use account_id, or use model_validate(params) which parses the normalized account field (e.g., create_media_buy, sync_creatives)

[ChrisHuie]

I don't think you can ever reach ToolError here for TypeAdapter errors. You should get a raw pydantic.ValidationError from FastMCP at server.py:986-988

Think this can possibly be removed since is already handled properly from ValidationError

[KonstantinMirin]

Agreed — TypeAdapter raises raw pydantic.ValidationError, never ToolError. The ToolError branch is dead code. Will remove as part of the retry rework (separate from this commit since the retry mechanism itself needs redesign — see discussion on the json.loads/json.dumps comment).

[KonstantinMirin]

Fixed in 5f012fb. Dropped the ToolError branch entirely — _should_retry now only checks isinstance(exc, ValidationError) + exc.title.startswith("call[").

FastMCP's TypeAdapter raises raw pydantic.ValidationError (confirmed by tracing function_tool.py:244-287). ToolError comes from our tool functions, after TypeAdapter has already passed.

[ChrisHuie]

Can you double check this? json.loads(json.dumps(normalized)) on these arguments I think just reproduces an identical dict already coming from FastMCP.

Also, if TypeAdapter rejects arguments due to a schema mismatch then just resending the same dict again would give the same rejection? The retry would only succeed if the behavior is non-deterministic but it isn't right...

It's early so may be missing something here

[KonstantinMirin]

You're right — MCP arguments are already plain JSON types so the round-trip is a no-op. The retry can never change the outcome. The intent was to bypass TypeAdapter when our Pydantic models (extra='ignore') would accept the args, but the mechanism doesn't achieve that. Options under discussion: (a) bypass TypeAdapter by calling tool.fn directly on retry, (b) recursive deep-stripping of nested unknowns using the tool's JSON Schema. Both are more involved — will address in a follow-up commit.

[KonstantinMirin]

Fixed in 5f012fb. Replaced the json round-trip with recursive deep stripping using the tool's JSON Schema.

The middleware now:

1. Gets the tool's full schema via tool.parameters
2. Walks the value alongside the schema tree
3. At each object level with additionalProperties: false, strips unknown properties
4. Handles anyOf/oneOf unions by picking the variant that preserves the most fields
5. Retries with cleaned arguments — TypeAdapter accepts, Pydantic models validate

This is schema-independent from AdCP — strips against what our tool actually accepts. 40 unit tests covering postconditions, boundaries, invariants (idempotency, known-fields-never-removed), and real-world schemas.

[ChrisHuie]

I assume code and error_code will be expanded in the future?

[KonstantinMirin]

The or code in error_str fallback is intentional — transports format errors differently (MCP wraps in ToolError string tuple, A2A uses JSON-RPC, REST uses HTTP body). Stricter matching belongs in transport-specific result parsing inside TransportResult.error, not in this assertion helper. No concrete expansion planned beyond what's here — it already handles cross-transport comparison correctly.

[ChrisHuie]

json.dumps(error.details) in the future could crash inside exception handler if something that isn't JSON-serializable is passed like {"timestamp": datetime.now()}

Maybe this for more defensive coding:
try: details_json = json.dumps(error.details) except (TypeError, ValueError): details_json = None

[KonstantinMirin]

Fixed in 0b7a695 — wrapped in try/except (TypeError, ValueError) as suggested. Falls back to details_json = None so the original error always propagates.

[ChrisHuie]

Is this meant to be scoped to only the get_products tool because right now is global since not tool scoped?

[KonstantinMirin]

It's intentionally global — promoted_offerings → catalogs maps the AdCP v2.5 field name to current. No tool accepts catalogs as a top-level parameter today, so it's forward-mapping for when the spec adds it. In practice, the value gets stripped by strip_unknown_params in production. Leaving as-is (documented).

Scoped campaign_ref → buyer_campaign_ref to create_media_buy only — that's the only tool with buyer_campaign_ref in its signature.

[KonstantinMirin]

Fixed in 0b7a695.

Root cause: _handle_get_media_buys_skill used manual parameters.get("account_id") which returned None after normalization converted it to account: {account_id: ...}. The handler bypassed model validation entirely.

Fix:

• A2A handler now uses GetMediaBuysRequest.model_validate(params) and calls _get_media_buys_impl directly (same pattern as update_performance_index)
• GetMediaBuysRequest gains account: LibraryAccountReference | None alongside legacy account_id
• _impl checks both req.account and req.account_id → raises "not yet supported" for either
• MCP/raw wrappers updated: function signature uses account: dict | None — normalization handles legacy callers transparently
• Removed the now-unused get_media_buys_raw import from A2A server

[KonstantinMirin]

UPD: This is completed

Re: extra middleware tests request —

The retry mechanism is under redesign (see thread on mcp_compat_middleware.py:104). Will add the requested tests once the retry approach is settled:

• `call_next` raises ValidationError in production → retry behavior
• `_should_retry` returns False in development → already tested (test_mcp_compat_middleware.py:134)
• `_get_known_params` returns correct tool schemas → already tested (test_mcp_compat_middleware.py integration tests)
• non-retry exceptions propagate → already tested (test_mcp_compat_middleware.py:161)