Skip to content

feat(server): advertise outputSchema on tools/list (JS parity)#396

Draft
bokelley wants to merge 2 commits intomainfrom
claude/issue-386-output-schema-tools-list
Draft

feat(server): advertise outputSchema on tools/list (JS parity)#396
bokelley wants to merge 2 commits intomainfrom
claude/issue-386-output-schema-tools-list

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 3, 2026

Closes #386

Summary

tools/list now carries outputSchema per ADCP tool, matching the TypeScript SDK's behaviour. Pydantic response types from adcp.types drive the schemas at import time via _generate_pydantic_output_schemas(), parallel to the existing _generate_pydantic_schemas() for inputSchema.

Four files changed:

  • src/adcp/server/mcp_tools.py — adds _generate_pydantic_output_schemas(), _PYDANTIC_OUTPUT_SCHEMAS, and _apply_pydantic_output_schemas() which writes outputSchema into every entry in ADCP_TOOL_DEFINITIONS at import time.
  • src/adcp/server/serve.py_register_tool gains an output_schema kwarg; _register_handler_tools extracts tool_def.get("outputSchema") and passes it through. The per-tool schema replaces FastMCP's generic dict inference in fn_metadata.output_schema (the tools/list advertisement source), while output_model (the runtime structuredContent path) is left unchanged.
  • src/adcp/server/test_controller.py — adds structured_output=True to the Tool.from_function call so FastMCP produces a non-None output_model (required by the assert in FuncMetadata.convert_result), then overrides output_schema with the spec-accurate schema from ADCP_TOOL_DEFINITIONS.
  • tests/test_mcp_schema_drift.py — 5 new drift tests mirror the inputSchema coverage: every-tool mapping, byte-level drift, $ref-free guarantee, and spot-checks of actual known fields.

Design decisions:

  • anyOf union response types (e.g. CreateMediaBuyResponse = Success | Error) are included — outputSchema is informational and anyOf is valid MCP contract for what a tool may return, unlike the inputSchema generator which skips them because anyOf confuses some MCP input-parsing clients.
  • output_model is intentionally kept as FastMCP's generic dict-accepting model for runtime structuredContent population. Per MCP §5.4.3, server-side enforcement that structuredContent conforms to outputSchema is a SHOULD; the existing opt-in validation config in create_tool_caller already handles that for callers who want it.

Known pre-existing issue (not introduced by this PR)

Several tools with discriminated-union schemas have discriminator.mapping string values like "all": "#/$defs/PublisherPropertySelector1" that reference $defs entries. After _inline_refs flattens those entries, the mapping strings become dangling pointers. This is identical to the pre-existing behaviour in inputSchema for ~16 tools. Tracked separately; this PR does not regress or worsen the issue.

What was tested

  • pytest tests/test_mcp_schema_drift.py — all 24 tests pass (5 new output schema tests included)
  • pytest tests/ (non-integration) — 2711 passed, 17 skipped, 0 failures
  • ruff check src/adcp/server/{mcp_tools,serve,test_controller}.py tests/test_mcp_schema_drift.py — clean
  • mypy src/adcp/server/{mcp_tools,serve,test_controller}.py — clean

Pre-PR review:

  • code-reviewer: approved — no blockers; spot-check test strengthened to verify actual field names (follow-up commit); pre-existing discriminator.mapping issue noted above
  • ad-tech-protocol-expert: approved — non-breaking per MCP spec 2025-03-26; anyOf for union responses is canonical; runtime vs. advertised schema separation acceptable per §5.4.3

Nits (not fixed):

  • Thin MCP codegen tools may silently collapse anyOf output schemas to any — client quality gap, not a protocol violation.
  • The _fallback path in test_controller.py is defensive; comply_test_controller will always have an output schema in practice.

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_019UpkNNacQ4QS8zcVTRfFQV

`tools/list` now carries `outputSchema` per ADCP tool, matching the
TypeScript SDK's behaviour.  Pydantic response types from `adcp.types`
drive the schemas via `_generate_pydantic_output_schemas()`, parallel to
the existing `_generate_pydantic_schemas()` for `inputSchema`.

Key design decisions:
- anyOf union response types (e.g. CreateMediaBuyResponse) are included —
  outputSchema is informational and anyOf is valid MCP contract here,
  unlike the inputSchema generator which skips them.
- `_inline_refs` flattens all $ref nodes for client compatibility.
- `_register_tool` accepts a new `output_schema` kwarg; the per-tool
  schema replaces FastMCP's generic dict inference in fn_metadata while
  the runtime `output_model` (permissive dict acceptor) is preserved so
  structuredContent keeps working.
- `register_test_controller` gains `structured_output=True` so its
  fn_metadata has a non-None `output_model` required by FastMCP's
  assertion in `convert_result`.

https://claude.ai/code/session_019UpkNNacQ4QS8zcVTRfFQV
…ld names

Mirror the inputSchema spot-check pattern: assert specific known fields
('products' in get_products, 'anyOf' in create_media_buy, 'adcp' +
'supported_protocols' in get_adcp_capabilities) rather than just
isinstance checks. Structural regressions on response models now surface
in the spot-check test, not only in the full drift comparison.

https://claude.ai/code/session_019UpkNNacQ4QS8zcVTRfFQV
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(server): advertise outputSchema on tools/list (JS parity)

2 participants