LearnNewsExamplesServices
Frontmatter
titlefix(mcp): emit items for array schemas to satisfy strict validators (#10064)
authortobiu
stateMerged
createdAtApr 18, 2026, 7:51 PM
updatedAtApr 18, 2026, 7:58 PM
closedAtApr 18, 2026, 7:58 PM
mergedAtApr 18, 2026, 7:58 PM
branchesdev ← claude/admiring-easley-fc4b56
urlhttps://github.com/neomjs/neo/pull/10065
Merged
tobiu
tobiu commented on Apr 18, 2026, 7:51 PM

Summary

GitHub Copilot's MCP client rejected mcp_neo_mjs-neura_call_method with:

Failed to validate tool ... array type must have items

Root-caused to a three-stage interaction:

  1. neural-link/openapi.yaml declared CallMethodRequest.args as type: array without an items field (four total occurrences across neural-link; other four MCP servers clean).
  2. OpenApiValidator.mjs handled the missing items by producing z.array(z.any()).
  3. zod-to-json-schema with target: 'openApi3' silently strips items from z.array(z.any()), emitting {"type":"array"}. Strict validators (Copilot) reject this; permissive ones (Claude, Cursor) accept it, which is why the bug was invisible until a Copilot user surfaced it.

Fix (two layers)

Layer 1 — ai/mcp/validation/OpenApiValidator.mjs Both the array fallback and the catch-all now return z.unknown() instead of z.any(). Runtime validation is identical (both accept anything); the only observable difference is in zod-to-json-schema emission — z.unknown() preserves items: {}, satisfying strict validators.

Layer 2 — ai/mcp/server/neural-link/openapi.yaml

  • CallMethodRequest.args → added items: {} (heterogeneous method args)
  • /data/store/inspect response filters, sorters, items → added items: {type: object}

Verification

New regression test test/playwright/unit/ai/mcp/validation/OpenApiValidatorCompliance.spec.mjs:

  • Asserts z.array(z.unknown()) emits items under the openApi3 target (direct guard on Layer 1).
  • Walks the emitted input + output JSON Schemas for every operation on all five MCP servers, failing if any array node lacks items.
$ npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/mcp/validation/OpenApiValidatorCompliance.spec.mjs
  6 passed (672ms)

Orthogonality to prior work

  • #10043 (closed) removed the hardcoded z.array(z.string()) — unrelated prior regression.
  • #9837 (open) covers nested array schemas defaulting to strings — also separate.
  • This PR covers the residual third case: arrays with no items key at all.

Avoided Traps

  • Not reintroducing hardcoded z.array(z.string()) fallback — would regress typed args.
  • Not post-processing emitted JSON Schema to inject items — adds a second serialization layer when z.unknown() achieves the same outcome natively in Zod.
  • Not silently widening every YAML array — keeps the YAML as the source of intent; Layer 1 is the safety net, not the primary contract.

Origin Session ID

97343e6e-9d1c-4170-b3a5-6e58d41511b6

Resolves #10064

tobiu
tobiu commented on Apr 18, 2026, 7:57 PM

Input from Claude Opus 4.7 (Claude Code):

❋ # PR Review Summary

Status: Comment (Self-Review)

Self-Review Opening: Self-review of #10064. I chose a two-layer fix (defense-in-depth in OpenApiValidator + source-of-truth in the YAML) over either one in isolation, because mid-verification I discovered the YAML fix alone was insufficient — items: {} recurses through buildZodSchemaFromNode to the catch-all, which was still z.any(). That forced the Layer 1 scope to cover both the array fallback and the final catch-all. Key trade-offs and gaps noted below.

📊 Evaluation Metrics

  • [ARCH_ALIGNMENT]: 88 — Respects the OpenAPI-as-source-of-intent principle (YAML is primary contract, validator is safety net). z.unknown() over z.any() aligns with the "prefer the stricter type when runtime behavior is identical" principle already established in the file.
  • [CONTENT_COMPLETENESS]: 82 — Both inline comments in the validator explain the why (zod-to-json-schema openApi3 quirk) not just the what. JSDoc on the new test helper cites the exact error and ticket. The validator module itself lacks a file-level @summary Anchor — not touched here since it predates this PR, but worth flagging (see [KB_GAP]).
  • [EXECUTION_QUALITY]: 85 — 6/6 tests green; sweep covers 34 operations across all 5 servers; caught the two-stage fallback during verification rather than post-merge. Loss: test couples directly to js-yaml + zod-to-json-schema rather than exercising ToolService.listTools() end-to-end. See [TOOLING_GAP].
  • [PRODUCTIVITY]: 95 — Ticket goal achieved: Copilot's call_method schema now emits items: {}. Zero scope creep into #9837 (adjacent ticket in the same file).
  • [IMPACT]: 70 — Unblocks an entire MCP host (GitHub Copilot) and hardens the validator against any future YAML gap in any of the five servers. Not framework-critical, but a real integration-surface fix.
  • [COMPLEXITY]: 35 — Three small touchpoints (+96/-3). The cognitive load was in diagnosing the three-stage pipeline interaction, not in the patch itself.
  • [EFFORT_PROFILE]: Quick Win — high ROI (restores a major harness), low code complexity.

🕸️ Context & Graph Linking

  • Target Epic / Issue ID: Resolves #10064
  • Related Graph Nodes: #10043 (closed — input schema hardcoding fix), #9837 (open — nested array items). This PR is the orthogonal third case and does not subsume #9837.
  • Origin Session ID: 97343e6e-9d1c-4170-b3a5-6e58d41511b6

🧠 Graph Ingestion Notes

  • [KB_GAP]: ai/mcp/validation/OpenApiValidator.mjs has per-function JSDoc but no file/module-level Anchor describing the three-stage YAML→Zod→JSON-Schema pipeline. Future agents hitting a similar class of bug (serialization quirk) would benefit from an Anchor at the top calling out "this file bridges OpenAPI to MCP-consumable JSON Schema; emission quirks live here". Out-of-scope for this fix.
  • [TOOLING_GAP]: (1) The regression test replicates the server's dependency chain (js-yaml + zod-to-json-schema) rather than exercising ToolService.listTools() directly. If the server swapped emitters, the test would still pass against the old library while shipping a broken schema. A stronger oracle would boot ToolService and validate the emitted tools with Ajv in strict mode. (2) test/playwright/unit/ai/mcp/server/memory-core/managers/ChromaManager.spec.mjs fails to import ai/mcp/server/memory-core/config.mjs in this worktree — unrelated to this PR but surfaces that config scaffolding is gitignored without a template fallback for worktree-isolated harnesses. (3) Authorization.spec.mjs has a pre-existing timeout on the "401 when no token provided" case — also unrelated.
  • [RETROSPECTIVE]: Two takeaways from my own process on this one. (a) Assign before implementing. I filed #10064 and started coding without running manage_issue_assignees add @me; tobiu flagged it as a minor process flaw. Feedback memory saved — next session will fold assignment into the same Ticket-First step as creation. (b) Trust-but-verify fallback paths. My first validator patch only touched the array-missing-items branch. If I had stopped there and relied on the YAML fix, the items: {} sentinel would have slipped through via the unrelated catch-all — an invisible regression. Running the emission through buildZodSchema end-to-end (not just reading the diff) caught it. Worth codifying: any time you change a schema-emission path, re-emit the actual artifact and diff it, don't just reason about it.

📋 Required Actions

  • Human QA confirms Copilot no longer errors on call_method (the user-facing trigger for this ticket).
  • (Optional, follow-up) File a lightweight ticket for the stronger Ajv-strict oracle test ([TOOLING_GAP] #1). Scope is small enough to bundle with whoever picks up #9837.

No block-level changes requested — the fix itself is complete and tested.

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