fix(server): collect stranded defs and memoize the non-compact schema modes

[bokelley]

Summary

Follow-up to #1507, which merged before these landed. The 8.x backport (#1512) went through its own review, and three findings apply to main too.

Stranded $defs survive the widening — and the orphan is stricter than the live arm

_drop_unreachable_defs runs inside _model_to_json_schema, before _widen_media_buy_output_schema_for_legacy_statuses repoints a union arm at its widened copy. Nothing collects the definition the arm left behind.

That is not just dead weight on the payload #1507 exists to shrink. The stranded twin still carries the narrow status: {"const": "completed"}, so a client that resolves definitions by model name rather than by following $ref — which the dotted $defs names invite — picks exactly the shape the widening replaces, and rejects a legitimate negotiated-3.0 status: "pending_creatives".

The widener now reports whether it fired, and unreachable definitions are collected again when it did. It also decides which arm to widen before copying, instead of cloning every arm and discarding all but one — #1507 shipped three *LegacyStatus definitions per tool with one referenced.

The documented escape hatch was also the expensive path

schema_mode="inline" and "defs" regenerate the whole Pydantic graph on every call, with no memoization. That is what #1507 tells a ref-averse adopter to set to restore the previous output shape, and it lands on per-request callers — ~7.5s per call for a 13-tool handler.

Schemas are now memoized per (kind, mode, tool) and deep-copied out, since a tool definition handed to a caller is an ordinary mutable JSON value.

mode	repeat call before	after
defs	7.5s	0.40s
inline	7.4s	1.05s

One claim in #1507 was too strong

#1507 justified keeping objects closed as "the only check a buyer has against a seller smuggling extra fields into a payload it feeds to a model." Where a Pydantic model sets extra="allow" — Product and most response roots — the advertised object is open in every mode, so that is not a filter a buyer can lean on. The property actually preserved is fidelity to the runtime model. Reworded everywhere it was stated.

Tests

The smuggling test in #1507 was vacuous: its sample omitted eight required fields, so it passed with the smuggled key removed, and pytest.raises could not tell the two apart. It now uses control_media_buy, whose success arm is both genuinely closed and genuinely drops an optional, and asserts the clean payload validates first so the unknown key is the only variable. Verified to fail under the reverted behaviour.

Also added: coverage that the non-compact escape hatch actually reaches output schemas (deleting that wiring left 175 tests green), and a sweep for dangling $refs across every advertised schema.

Reference seller tools/list: 1,368,877 bytes (down another ~38 KB from the stranded definitions).

🤖 Generated with Claude Code

---

Open workspace in Conductor

[aao-secretariat]

Ladon verdict: Approve

Approve.

Clean follow-up to #1507 applying three #1512 backport findings to main. Reviewer verified: the stranded-def fix re-runs _drop_unreachable_defs after the widener repoints an arm (correct ordering, reachability-safe), the deferred clone yields one *LegacyStatus def per matching arm, and the (kind,mode,name)-keyed schema cache is sound (handler-independent, deep-copied out). fix(server): is the right semver signal — all changes are underscore-prefixed internals with no public adcp.* shape change. Tests back every claim.

Decision-table walk:

• Row 1 (critical/high): no findings — skip.
• Row 2 (gated_paths): false — skip.
• Row 3 (high_risk deleted): only (modified) — skip.
• Row 4/5 (medium findings): none — skip.
• Rows 6–8: no prior escalate, no team gate, zero medium findings — skip.
• Row 9: approve.

High-risk flag is true (src/adcp/server/mcp_tools.py modified) but is a heuristic signal only; reviewer found no medium-or-higher concerns on the modified file, so the modification is presumed safe. review_decision: REVIEW_REQUIRED does not block because gated_paths is false.