Skip to content

fix: flatten MCP query tool schema so SDK serializes inputSchema correctly#2124

Open
brandon-pereira wants to merge 2 commits intomainfrom
brandon/fix-mcp-query-schema
Open

fix: flatten MCP query tool schema so SDK serializes inputSchema correctly#2124
brandon-pereira wants to merge 2 commits intomainfrom
brandon/fix-mcp-query-schema

Conversation

@brandon-pereira
Copy link
Copy Markdown
Member

@brandon-pereira brandon-pereira commented Apr 15, 2026

Summary

During code review on the original MCP PR (#2030), the hyperdxQuerySchema was refactored from a flat object into a z.discriminatedUnion() to reduce duplication and improve type safety. This inadvertently broke the tool's parameter schema — the MCP SDK's normalizeObjectSchema() only recognizes plain z.object() schemas and silently replaces anything else (discriminated unions, ZodEffects from .superRefine/.refine/.transform) with an empty {} in the tools/list response, making the tool unusable for MCP clients.

Changes:

  • Replaced z.discriminatedUnion() with a single flat z.object() that the SDK can serialize
  • Moved cross-field validation (e.g. "sourceId required for builder types", "sql required for sql type") into a standalone validateQueryInput() function called at runtime in the handler
  • Added a regression test that calls tools/list and asserts all expected properties (displayType, sourceId, select, where, sql, connectionId, etc.) are present in the serialized inputSchema

How to test locally or on Vercel

  1. Start the dev stack with yarn dev
  2. Connect any MCP client and call tools/list — verify hyperdx_query exposes all properties (displayType, sourceId, select, where, sql, etc.) instead of an empty schema
  3. Run the query tool test suite: cd packages/api && yarn ci:unit src/mcp/__tests__/queryTool.test.ts

References

The MCP SDK's normalizeObjectSchema() only recognizes plain z.object()
schemas. Using z.discriminatedUnion() wraps the schema in a type the SDK
silently replaces with an empty schema in the tools/list response --
making the tool unusable for MCP clients.

Replaced the discriminated union with a flat z.object() for SDK
serialization and a separate validateQueryInput() function for
cross-field validation at runtime. Added a regression test that verifies
the schema properties are properly exposed via tools/list.
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Apr 15, 2026 5:54pm

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 15, 2026

🦋 Changeset detected

Latest commit: 680e7ff

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/api Patch
@hyperdx/app Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the review/tier-3 Standard — full human review required label Apr 15, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 15, 2026

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats
  • Production files changed: 2
  • Production lines changed: 168 (+ 27 in test files, excluded from tier calculation)
  • Branch: brandon/fix-mcp-query-schema
  • Author: brandon-pereira

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 15, 2026

PR Review

Fix for MCP SDK silently dropping non-plain z.object() schemas — well-described, targeted, and includes a regression test.

  • ⚠️ input.select!.map(...) (index.ts:97) — the ! non-null assertion is safe here because validateQueryInput returns early for builder types missing select, but TypeScript cannot prove that statically. Consider narrowing the type explicitly (e.g., checking input.select before the builder branch) rather than relying on the non-null assertion, to prevent a future refactor silently breaking the guarantee.

  • ⚠️ !data.select || data.select.length === 0 in validateQueryInput — the length === 0 branch is unreachable: z.array(...).min(1).optional() already rejects empty arrays before this runs. Not a bug, just dead code.

  • ✅ The core fix is correct — flattening to a plain z.object() with runtime cross-field validation in validateQueryInput is the right workaround for the MCP SDK limitation.

  • ✅ Regression test (schema serialization describe block) directly verifies the bug that was fixed.

  • ✅ Changeset included, validation error format matches existing handler patterns.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 15, 2026

E2E Test Results

All tests passed • 132 passed • 3 skipped • 1064s

Status Count
✅ Passed 132
❌ Failed 0
⚠️ Flaky 4
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-3 Standard — full human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant