feat(server): backport compact output schemas to 8.x so discovery is servable

[bokelley]

Summary

Backport of #1507 to release/8.x.

examples/seller_agent.py on this line answers MCP discovery with a 22,777,757-byte (21.7 MiB) tools/list for 12 tools. A buyer applying the AdCP default 4 MiB response budget (adcontextprotocol/adcp#8190) rejects that outright, so the agent looks unreachable rather than verbose — which is how adcp-client's python reference seller gate executes zero tracks against it. The "advertising 12 of 78 tools" line in the incident report is this branch.

After: 1,251,781 bytes (1.19 MiB), same 12 tools.

Cause

Not over-advertisement — the override filter already narrows the handler to the 12 tools it implements. _generate_pydantic_output_schemas called _model_to_json_schema(response_type, allow_root_union=True) with no schema_mode, so every output schema took that function's own "inline" default and expanded each shared $ref once per reference, while input schemas had been "compact" since they were introduced.

On this branch, list_creatives: inputSchema 14,247 bytes, outputSchema 4,802,383 bytes.

Output schemas now honour schema_mode, compact by default, so both sides of a tool definition have the same shape. Clients already had to resolve local $ref to read an inputSchema; schema_mode="inline" still restores the reference-free shape for those that cannot.

⚠️ The advertised wire shape changes

This is the one thing an 8.1.3 → 8.1.4 upgrader needs to read, so it is in the CHANGELOG as well as here.

• Output schemas gain ~530 local $ref sites and ~500 boolean property schemas (a dropped optional keeps its name bound to true) where there were previously none of either. Client code written against the fully inlined shape may need updating: schema["properties"][name]["type"] raises TypeError: 'bool' object is not subscriptable on a kept name, and a client that does not resolve local references cannot read the schema at all.
• Input schemas change less but do change: compaction is shared, so 38 nested objects across 11 tools now advertise additionalProperties: false where 8.1.3 advertised true. That tracks what the runtime models already enforced (extra="forbid") — server behaviour is unchanged — but a client that pre-validates its own request against the advertised schema and sends a vendor extension on, say, BusinessEntity now fails locally where 8.1.3 let it through and the server rejected it.

schema_mode="inline" on create_mcp_server() / serve() / MCPToolSet restores the previous shape.

Release class: minor (8.2.0)

Filed as feat: rather than fix: on a maintainer decision. The advertised discovery contract changes for every non-pinned seller on this line, and 8.x does not ship the buyer-side response cap — so an 8.x buyer may have been parsing the 21.7 MiB payload successfully, and is exactly who the shape change affects.

Changelog-facing summary:

• outputSchema now uses local $defs/$ref and boolean property schemas (a dropped optional keeps its name bound to true) where it previously inlined everything. A client that parses outputSchema must resolve local references. One that indexes into a property as an object — schema["properties"][name]["type"] — raises TypeError on a kept name.
• inputSchema gains additionalProperties: false on 38 nested objects across 11 tools, matching what the runtime models already enforced.
• tools/list for the reference seller drops from ~21.7 MiB to ~1.19 MiB.
• schema_mode="inline" on create_mcp_server() / serve() / MCPToolSet restores the previous shape.

Scope

Per the maintainer: the schema fix plus its size test. The buyer-side response cap from #1504 is not backported.

Three things came along because the switch requires them, not as scope creep:

1. _widen_media_buy_output_schema_for_legacy_statuses walks anyOf arms expecting inline properties, so against a $ref arm it silently no-ops — advertising a schema that rejects a legitimate negotiated-3.0 status: "pending_creatives". It now resolves refs and widens a copy, then collects the definition the repointed arm stranded (the stranded twin still carries the narrow const: "completed").
2. _prune_to_required_fields flipped additionalProperties: false → true whenever it dropped an optional. Harmless while outputs were inline; with compact outputs it would have advertised objects as accepting keys the runtime model rejects. A dropped optional now keeps its name bound to true instead.
3. Non-compact modes regenerate the whole Pydantic graph per call, and this line has four per-request get_tools_for_handler callers including the pre-auth agent-card route. Adding outputs there would have taken it from ~1.9s to ~7.1s per request — and schema_mode="inline" is the documented remediation for the shape change, so the remediation was the DoS path. Schemas are now memoized per (kind, mode, tool): repeat calls drop to 0.35s (defs) and 0.44s (inline).

Deliberately not carried from #1507: the startup payload-size warning, the advertised_tools declaration on DemoSeller, the A2A agent-card _include_schemas=False fix (the default path got ~18x cheaper on its own), the reworded unregistered-subclass warning, and the docs prose.

Review

A code-reviewer and a security-reviewer pass ran against the first commit. Both confirmed the port is mechanically faithful — _prune_to_required_fields and _model_to_json_schema are byte-identical to main after it — and that nothing narrows: a differential over all 78 compact output schemas against their defs-mode counterparts found zero nodes where a closed object lost a key or a payload the runtime model accepts would be rejected.

What they caught, and what changed:

Finding	Fix
The anti-smuggling test was vacuous — its sample omitted 8 required fields, so it passed with the smuggled key removed	Rewritten on control_media_buy, whose arm is genuinely closed and genuinely drops an optional; asserts the clean payload validates first
The claim it guarded was overstated: Product and most response roots are extra="allow", open in every mode	Reworded to schema fidelity, not a buyer-side filter
_prune_to_required_fields docstring described the behaviour the port replaced	Updated
Stranded $defs survive the widening, and the orphan is stricter than the live arm	Collected after widening
Non-compact modes are uncached on a pre-auth path, and are the documented remediation	Memoized
schema_mode still documented as "input discovery shape"; non-default cost undocumented	Both fixed
The escape hatch had no test — deleting its wiring left 175 tests green	test_non_default_schema_modes_reach_output_schemas_too
Nothing swept for dangling $refs	test_advertised_schemas_have_no_dangling_references
Input-schema change was undeclared on a stable line	CHANGELOG

Accepted with reason: compact $defs keys publish generated-model names pre-auth — the already-shipped compact inputSchema publishes 18+ of exactly that form, so nothing here is worse than 8.1.3 already serves.

Tests

tests/test_discovery_payload_size.py (new, 13 tests) plus ref-aware updates to three existing files. Reverting the output schema_mode default to "inline" fails 4; reverting the closure fix fails the smuggling test; deleting the escape-hatch wiring fails its own test — all verified.

🤖 Generated with Claude Code

[aao-secretariat]

Ladon verdict: Comment (human reviewer recommended)

Comment — 1 medium finding; wire-shape change under non-breaking prefix needs a human eye.

This is the #1507 backport to release/8.x: compact output schemas, $ref-aware media-buy widening, prune-time re-admit-as-true, and per-(kind,mode,tool) memoization. Core logic reviewed and found correct — widening is idempotent and pairs with _drop_unreachable_defs to collect the stranded narrow twin; _prune_to_required_fields keeps closed objects closed while preserving the field inventory; the memo cache is keyed correctly and returns deep copies; the alternate path is scoped to non-pinned handlers only. Tests are non-vacuous (anti-smuggling on control_media_buy, escape-hatch, dangling-ref sweep).

Medium findings

• src/adcp/server/mcp_tools.py:2239 — Advertised outputSchema wire shape changes and the ref-free-output guarantee is removed, shipping under a fix:/patch prefix. Mitigated by the inline escape hatch and CHANGELOG prose, but a human should confirm the patch-level semver signal is intended for a wire-shape change on the public surface.

Why comment and not approve: the repo's semver-signal rule treats a response-model shape change under a non-breaking prefix as potentially a high finding (release-please cuts a minor from fix:, so a break could ship without a major). The reviewer graded this Medium because the escape hatch and CHANGELOG mitigate, but confirmation that patch is the intended semver signal warrants a human look before merge.

Why not escalate: high_risk is true only via a (modified) match on src/adcp/server/mcp_tools.py, and the single medium is not in data-loss/schema/infra category (row 4), so row 5 is the only candidate — a (modified) high-risk file WITH a medium finding does fire row 5.

Medium findings

• src/adcp/server/mcp_tools.py:2239 — Advertised outputSchema wire shape changes under fix:/patch prefix

[aao-secretariat]

Ladon verdict: Approve

Approve — clean backport of #1507 to release/8.x with prior feedback resolved.

The prior medium finding (outputSchema wire-shape change under a fix:/patch prefix at mcp_tools.py:2239) has been addressed: the commit now ships as feat: (cutting 8.2.0 minor) with a full CHANGELOG "Advertised schema shape" entry and a documented escape hatch (schema_mode="inline"). Response models themselves are unchanged, so the mandatory response-deserialization semver trigger does not strictly fire; the breaking-vs-minor classification is an explicit, documented maintainer call.

The three correctness-sensitive edits were verified: _widen_media_buy_output_schema_for_legacy_statuses now resolves $ref arms, clones/repoints under {name}LegacyStatus, and strands/collects the original def via _drop_unreachable_defs (mcp_tools.py:108-122); _prune_to_required_fields re-admits dropped names via setdefault keeping closed objects closed (mcp_tools.py:1873-1894); and _alternate_schemas memoization is bounded, keyed by kind/mode/name, deep-copied on return, and does not touch the version-pinned bundled-wire-schema branch. Tests are thorough (dangling-ref sweep, smuggling closure, escape-hatch wiring, real-response validation).

Decision table: no critical/high/medium findings (rows 1, 4-6, 8 do not fire); gated_paths is false (row 2 n/a); high_risk is true but the only reason is a (modified) file with no medium-or-higher finding, so rows 3 and 5 do not fire; no no-auto-approve team match (row 7 n/a). Prior decision was comment, not escalate, so row 6 is irrelevant. Falls through to row 9.