LearnNewsExamplesServices
Frontmatter
title>-
authortobiu
stateMerged
createdAtApr 18, 2026, 9:14 PM
updatedAtApr 18, 2026, 10:26 PM
closedAtApr 18, 2026, 9:20 PM
mergedAtApr 18, 2026, 9:20 PM
branchesdev ← agent/9837-output-schema-drift
urlhttps://github.com/neomjs/neo/pull/10067
Merged
tobiu
tobiu commented on Apr 18, 2026, 9:14 PM

Summary

Strict MCP clients (notably GitHub Copilot) have been crashing on output-heavy Neural Link / Memory Core tools with:

Error: Structured content does not match the tool's output schema: data/result/0 must NOT have additional properties

Every z.object(...) emitted from OpenAPI response schemas became additionalProperties: false in the JSON Schema, but the underlying services routinely return fields the OpenAPI contract forgot to declare (e.g. ConnectionService.sessionData returns {appName, connectedAt, logs, sessionId} while the schema declared a different six-field shape). The drift was invisible to lenient clients (Claude Desktop, Cursor) and became fatal the moment #10043 made outputSchemas spec-compliant, activating strict client-side validation.

Fix (Output-side defense-in-depth)

In ai/mcp/validation/OpenApiValidator.mjs:

  • buildZodSchemaFromNode(...) now accepts a {lenient} opt threaded through the entire recursion.
  • When lenient && !schema.additionalProperties, z.object(shape) is wrapped in .passthrough() — zod-to-json-schema then emits additionalProperties: true.
  • buildOutputZodSchema(...) passes lenient: true throughout and makes the {result: ...} envelope passthrough as well (covers the non-object-response wrapping path added by #10043).
  • Input side stays strict: buildZodSchema(...) does not pass the flag — agents calling tools with unknown top-level fields still fail validation, preserving contract safety.

Asymmetry matches the fault model: server implementations drift faster than OpenAPI contracts; client contracts stay pinned.

Scope — before / after

Output-side strict-object counts across every operation on every MCP server:

Server Total ops Strict-obj before Strict-obj after Strict-item before Strict-item after
file-system 6 2 0 0 0
github-workflow 18 17 0 3 0
knowledge-base 8 7 0 3 0
memory-core 17 17 0 5 0
neural-link 34 31 0 11 0
total 83 74 0 22 0

74 tools unblocked. Verified input side remains strict (e.g. call_method input additionalProperties stays false).

Verification

Extended test/playwright/unit/ai/mcp/validation/OpenApiValidatorCompliance.spec.mjs with:

  • Direct zod-emission guard: z.object(...).passthrough() flips additionalProperties to true.
  • Per-server output-tolerance check: no emitted output schema object may carry additionalProperties: false anywhere in its tree.
  • Per-server input-strictness guard: every root input schema must keep additionalProperties: false. Regression guard against accidentally leaking the output-leniency into the input path.
$ npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/mcp/validation/
  17 passed (649ms)

(2 unit guards + 5 servers × 3 per-server assertions = 17 tests)

Three-layer root cause (from self-repair diagnosis)

  1. OpenAPI schema drift: declared response fields diverge from service returns over time.
  2. zod-to-json-schema quirk: z.object(...) defaults to additionalProperties: false on emission.
  3. #10043 activation: wrapping non-object responses in {result: ...} made outputSchemas spec-compliant (type: object), which flipped client-side strict validation on — exposing the latent drift.

Full forensic trace captured in #9837's re-purposed body.

Avoided Traps

  • Not reverting #10043 — that change correctly satisfies the MCP spec.
  • Not flipping zod-to-json-schema globally. additionalProperties: false is still correct for inputs; agents passing unknown fields should fail fast.
  • Not auditing every response schema inline. 74 tools is too large a surface for one PR. This change tolerates drift; a follow-up audit ticket (Step 3 in #9837's body) will realign schemas at the source.

Commit History

  • fix(mcp): tolerate extra properties on output schemas via lenient buildZodSchemaFromNode recursion + buildOutputZodSchema passthrough envelopes

Follow-up (not this PR)

Schema-vs-implementation audit per Step 3 of #9837's body — realign declared response shapes with actual service returns so the leniency is defense-in-depth rather than load-bearing.

Origin Session ID

97343e6e-9d1c-4170-b3a5-6e58d41511b6 (diagnosed via self-repair skill)

Resolves #9837

tobiu
tobiu commented on Apr 18, 2026, 9:18 PM

Input from Claude Opus 4.7 (Claude Code):

❋ # PR Review Summary

Status: Comment (Self-Review)

Self-Review Opening: Self-review of #9837 (re-purposed). I chose asymmetric validation (strict input / lenient output) over blanket additionalProperties: true because server implementations drift faster than OpenAPI contracts, but agents calling tools with wrong top-level keys should still fail fast. The {lenient} flag threaded through buildZodSchemaFromNode keeps the two paths honest. Trade-offs and honest gaps below.

Tech Debt Radar: deliberately not invoked. Change is confined to one validation file + its spec; surgical, not a fundamental framework shift. The three-layer root-cause analysis in #9837's body already surfaces the relevant adjacent context (#10043 activation + #10064 sibling).

📊 Evaluation Metrics

  • [ARCH_ALIGNMENT]: 92 — Clean asymmetric design. Context threaded via opts = {lenient} rather than duplicating the recursion. The .passthrough() wrapping respects existing additionalProperties declarations (only kicks in when OpenAPI didn't specify). Matches the fault model: server drift is the real problem, not client strictness.
  • [CONTENT_COMPLETENESS]: 90 — JSDoc on the new opts parameter explains why leniency exists only for outputs, with a direct reference to #9837. Inline comment in buildOutputZodSchema documents the input/output asymmetry rationale. PR body carries full forensic trace and cross-server sweep table.
  • [EXECUTION_QUALITY]: 87 — 17/17 green covering direct zod emission, per-server output tolerance, and per-server input-strictness regression guard. Honest gaps: (a) the input-strictness test only checks root object schemas, not nested ones — a future leak of lenient:true into the input path could slip past; (b) I didn't run the full npm run test-unit suite (only the validation subdirectory); (c) no live verification against Copilot — relying on emitted JSON Schema inspection as the proxy.
  • [PRODUCTIVITY]: 100 — Step 1 + Step 2 of #9837's plan delivered; Step 3 correctly deferred to a follow-up audit ticket rather than scope-crept into this PR.
  • [IMPACT]: 80 — Unblocks 74 tools across all 5 MCP servers for strict clients (Copilot + future stricter validators). Not framework-critical, but framework-wide.
  • [COMPLEXITY]: 45 — Small line count but non-trivial: recursion context threading, Zod/zod-to-json-schema emission quirks, asymmetric design thought.
  • [EFFORT_PROFILE]: Heavy Lift — high framework-wide impact, moderate cognitive complexity (diagnosis + cross-server audit + asymmetric design was the bulk of the work; the code change is surgical).

🕸️ Context & Graph Linking

  • Target Epic / Issue ID: Resolves #9837 (re-purposed)
  • Related Graph Nodes: Sibling #10064 (input-side of same MCP schema-validation family). Activated by #10043 (output-schema spec compliance). Originally cross-referenced by #9838.
  • Origin Session ID: 97343e6e-9d1c-4170-b3a5-6e58d41511b6

🧠 Graph Ingestion Notes

  • [KB_GAP]: zod-to-json-schema's default of emitting additionalProperties: false on every z.object(...) is a subtle quirk that trips up agents wiring OpenAPI → MCP. Worth a note in a README near ai/mcp/validation/ or in the AgentOS guide series — "if you produce Zod objects for MCP output schemas, wrap them in .passthrough() unless you know the service return shape is exhaustively declared." Would prevent the next agent from re-introducing this asymmetrically.
  • [TOOLING_GAP]: There is no automated check for OpenAPI-vs-implementation drift. A test or audit tool that samples actual service returns (via test-time introspection or a test double) and diffs against declared response shapes would catch this class of bug proactively, making the .passthrough() hotfix truly defense-in-depth rather than load-bearing. Sibling to the Step 3 audit ticket #9837's body proposes — worth filing explicitly.
  • [RETROSPECTIVE]: Three-layer forensic pattern is a reusable debugging template. When a tool "used to work" and suddenly doesn't, check: (1) the tool-surface change (none found here), (2) a recent activation commit that turned on stricter validation (#10043 here), and (3) the latent pre-existing divergence the activation exposed (schema vs implementation drift here). Don't stop at layer 1 when the agent reports a regression — the change that reveals a bug is often different from the change that introduced it. Similar mental model to git bisect: find the "surfacing" commit, then separately investigate what it surfaced.

📋 Required Actions

  • Human QA: open the devindex app in Copilot and confirm get_worker_topology / get_instance_properties / mutate_frontier / get_context_frontier no longer crash the tool registration.
  • (Optional follow-up) File the Step 3 audit ticket from #9837's body — schema-vs-implementation realignment. Worth tracking separately so the leniency is defense-in-depth rather than hiding real drift.
  • (Optional follow-up) File a ticket for an automated drift-detection check (per [TOOLING_GAP] above).

No block-level code changes — the final state is tight.

Handoff: awaiting human merge per pull-request-workflow.md §5 (Human-in-the-Loop). I will not self-merge.

tobiu
tobiu commented on Apr 18, 2026, 10:26 PM

Input from Claude Opus 4.7 (Claude Code):

❋ Post-merge correction (2026-04-18): the "Follow-up: schema-vs-implementation audit" in this PR's body is superseded.

Discussing follow-up tickets after merge, tobiu flagged that my original framing had a blind spot: I treated MCP tool outputs as typed contracts across the board, but Neural Link's runtime introspection tools — get_vnode, get_component_tree, get_instance_properties, VDOM/VNode accessors, inspect_component_render_tree, etc. — surface live application state where the response shape is as open-ended as the app itself. VDOM nodes carry arbitrary HTML attrs (aria-*, data-*, handlers, styles); component instances expose hundreds of configs per class.

For this class of tool, open shapes are the feature. The .passthrough() that landed here is architecturally correct design, not a hotfix awaiting a stricter fix. Auditing to narrow the schemas would fight the introspection pattern NL exists to provide.

Revised follow-up framing

My original "Follow-up: Schema-vs-implementation audit per Step 3 of #9837's body" bullet should be read as superseded. Any future audit work must be category-aware:

Category Examples Correct strategy
Closed contracts healthcheck, GitHub workflow ops, KB queries with fixed result shape Narrow schemas + audit drift. additionalProperties: false is a legitimate contract.
Runtime introspection Neural Link's get_vnode, get_component_tree, get_instance_properties, query_vdom, query_component Open shapes by design. .passthrough() is correct, not a hotfix.

The .passthrough() change here protects both categories simultaneously (strict input / lenient output) — the asymmetric design is correct for the closed-contract case (where drift is a bug to be audited later if observed) AND for the runtime-introspection case (where openness is inherent). The framing of this PR as "defense-in-depth while waiting for Step 3" was wrong in retrospect; it's actually "architecturally correct for introspection; safety net for contracts."

Saved as durable feedback (feedback_mcp_output_category_split.md) so future sessions don't re-propose the same audit trap.

Cross-posted to #9837 for symmetry.