Skip to content

feat(schema): add MCP 2026 JSON Schema projections - #6070

Open
bokelley wants to merge 1 commit into
mainfrom
review-mcp-spec-support
Open

feat(schema): add MCP 2026 JSON Schema projections#6070
bokelley wants to merge 1 commit into
mainfrom
review-mcp-spec-support

Conversation

@bokelley

@bokelley bokelley commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • generate compact, self-contained JSON Schema 2020-12 input/output artifacts for every canonical AdCP tool under the MCP 2026-07-28 projection path
  • preserve v3 draft-07 validation semantics while enforcing local-reference integrity and AdCP-defined defensive depth, object-count, and compact-byte bounds
  • enter Changesets beta pre-mode for 3.2.0-beta.0 and document the 3.2 projection / 4.0 canonical migration boundary
  • keep MCP projection artifacts out of the legacy draft-07 SDK validation overlay

Why

MCP 2026-07-28 defaults embedded tool schemas to JSON Schema 2020-12, requires implementations to support that dialect, and recommends its use; explicit alternative dialects remain allowed. AdCP v3 remains canonically draft-07. This projection provides MCP-compatible discovery artifacts without changing the v3 payload contract or prematurely introducing 4.0 contract tightening.

The artifacts are downloadable and opt-in for server tools/list registration; publishing them does not alter runtime tool registration automatically.

Validation

  • schema build: 64 tools, 128 projected schemas, 10.17 MiB compact total, 845 KiB largest
  • MCP projection tests: 8/8
  • schema validation: 20/20 across 648 schemas
  • composed-schema validation: 118/118
  • schema build regressions: 27/27
  • pre-commit repo tests: 1,025/1,025
  • pre-commit server tests: 4,630 passed, 30 skipped
  • current and released 3.0 storyboard matrices: all tenant floors passed
  • Changesets resolves the prerelease to 3.2.0-beta.0

Expert review

Reviewed by MCP protocol, deep implementation, and Node.js test/release experts. Their findings on artifact size, runtime claims, draft-07 $ref sibling semantics, full-document bounds, source-dialect drift, JSON Pointer rewriting, and SDK cache isolation were addressed; no merge blockers remain.

Local environment note

The existing typecheck currently reports server/src/utils/otel-logs.ts(92,5) against the installed dependency graph; this branch does not modify that file or package-lock.json. PR CI remains authoritative.

@bokelley
bokelley force-pushed the review-mcp-spec-support branch from d0a5a31 to d33c637 Compare July 28, 2026 13:49
@bokelley
bokelley force-pushed the review-mcp-spec-support branch from d33c637 to 898cc89 Compare July 30, 2026 01:16
@bokelley
bokelley marked this pull request as ready for review July 30, 2026 02:46
const projectedSchema = readJson(path.join(PROJECTION_DIR, relativePath));
const validateSource = draft07.compile(compactSource);
const validateProjected = draft2020.compile(projectedSchema);
const parityCorpus = sourceSchema.examples?.length ? sourceSchema.examples : [{}];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: The parity comparison — the one behavioral guard on the PR's load-bearing "semantics-preserving, does not tighten" claim — is nearly a no-op in the common case. It runs only the smallest schema per protocol (representatives keyed on min bytes, L271), and the corpus is sourceSchema.examples?.length ? sourceSchema.examples : [{}]. Verified: request schemas almost never carry top-level examples (get-signals-request.json has none), so the representatives resolve to [{}], which both dialects reject identically for any type: object schema with required fields. That instance never exercises the accept path or a rejecting boundary, so a tightening regression on a complex schema (an additionalProperties/prefixItems/dependentRequired edge) would pass parity silently and ship to adopters. Anti-tightening rests on the transformer unit tests, not this comparison. Feed a real accept/reject corpus (or at least the schemas' own valid examples plus a mutated-invalid variant) across more than the smallest-per-protocol schema.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated paths touched without required approval.

This PR adds a semantics-preserving JSON Schema 2020-12 MCP tool-schema projection (new scripts/mcp-schema-projection.cjs, wired into build:schemas), plus docs, a minor changeset, and a full-suite test. The transformer work is careful and unit-tested, and the reviewer surfaced no critical/high findings.

However, gated_paths is true and review_decision is REVIEW_REQUIRED (not APPROVED), so the hard approval gate (decision rule row 2) forces escalation regardless of how clean the diff is.

Escalation reasons

  • .changeset/pre.json (added) and static/schemas/source/core/forecast-dimension-geo.json (modified) are under the hard approval gate (## Gated Paths). Human/CODEOWNERS review is required — review_decision is REVIEW_REQUIRED, not APPROVED.

Medium findings (non-blocking)

  • tests/mcp-schema-projection.test.cjs:290 — Parity test barely validates the anti-tightening guarantee (nearly a no-op for schemas without top-level examples). Worth strengthening but not blocking.

The forecast-dimension-geo.json required-sibling removal was reviewed as semantics-preserving (necessitated by the fail-closed $ref-validation-sibling guard). Once a human/CODEOWNERS approval lands, this can proceed through the normal decision table.

Medium findings

  • tests/mcp-schema-projection.test.cjs:290 — Parity test barely validates the anti-tightening guarantee

Why human review

  • Touches gated paths .changeset/pre.json (added) and static/schemas/source/core/forecast-dimension-geo.json (modified) while review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required before merge.
  • This PR touches a path under a hard, non-overridable approval gate (.changeset/pre.json (added) matches .changeset/pre.json; static/schemas/source/core/forecast-dimension-geo.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant