Skip to content

fix(memory): add 'type' to schemas and use passthrough for Gemini CLI… - #3149

Closed
ericnjurio wants to merge 2 commits into
modelcontextprotocol:mainfrom
ericnjurio:fix/memory-gemini-cli-compatibility
Closed

fix(memory): add 'type' to schemas and use passthrough for Gemini CLI…#3149
ericnjurio wants to merge 2 commits into
modelcontextprotocol:mainfrom
ericnjurio:fix/memory-gemini-cli-compatibility

Conversation

@ericnjurio

@ericnjurio ericnjurio commented Dec 23, 2025

Copy link
Copy Markdown

Title
fix(memory): enhance schema compatibility for strict JSON validators (like Gemini CLI)

Description
This PR updates the memory server's Zod schemas to improve compatibility with MCP clients that enforce strict JSON schema validation, such as the Google Gemini CLI.

The changes include:

  1. Formally declaring the optional type property in EntitySchema and RelationSchema.
  2. Enabling .passthrough() on these schemas to allow for internal or future properties without breaking strict validation.

Server Details

  • Server: memory
  • Changes to: tools (Output schemas for read_graph, search_nodes, and open_nodes)

Motivation and Context
Strict MCP clients like the Gemini CLI perform rigorous validation on tool outputs. The memory server includes a type field in its internal JSONL storage which is subsequently returned in tool outputs. Since this field was not declared in the tool's output schema, Gemini CLI rejected the response with an "additional properties not allowed" error (code -32602).

By declaring the type field and using .passthrough(), we ensure the server remains robust and compatible with both permissive and strict clients.

How Has This Been Tested?
Tested locally using google-gemini-cli:

  • Verified that read_graph no longer triggers validation errors and returns the full graph.
  • Verified that search_nodes and open_nodes successfully return structured content.
  • Confirmed that data remains intact in the memory.jsonl file.

Breaking Changes

  • No. This change is additive and increases tolerance for existing data structures.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the [MCP Protocol Documentation (https://modelcontextprotocol.io)
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly (N/A - no user-facing config changes)
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally (Need to run npm test)
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options (N/A)

Additional context
The type discriminator is essential for the server's internal logic when parsing the JSONL file. Formalizing its presence in the communication schema resolves the "hidden property" conflict with strict validators.

@diffray-bot

Copy link
Copy Markdown

Changes Summary

This PR fixes a schema validation compatibility issue with the memory server by adding the optional 'type' property to EntitySchema and RelationSchema, and enabling .passthrough() on both schemas. This resolves 'additional properties not allowed' errors when strict JSON validators like Gemini CLI validate tool outputs that include the internal 'type' discriminator field.

Type: bugfix

Components Affected: memory-server, EntitySchema, RelationSchema, Entity interface, Relation interface

Architecture Impact
  • Coupling: The schemas now explicitly declare the internal 'type' field that was already present in the JSONL storage format, improving consistency between storage format and API contract

Risk Areas: The .passthrough() modifier allows any additional properties through validation, which could mask future schema drift or unexpected data, Input validation now accepts 'type' field from external clients, though it's marked as optional

Suggestions
  • Consider if .passthrough() is too permissive - an alternative would be to only declare the 'type' field without passthrough, maintaining stricter validation on other properties
  • Consider adding a test case that validates the schema works correctly with both permissive and strict validators

Full review in progress... | Powered by diffray

Comment thread src/memory/index.ts

// We are storing our memory using entities, relations, and observations in a graph structure
export interface Entity {
type?: string;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we just change this to the const "entity" and relation to "relation"?

Comment thread src/memory/index.ts Outdated
Comment thread src/memory/index.ts
entityType: z.string().describe("The type of the entity"),
observations: z.array(z.string()).describe("An array of observation contents associated with the entity")
});
}).passthrough();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not convinced we should have this passthrough() declaration - if we already have type declared does this sort out the schema situation? I worry that passthrough could lead to silent errors & schema drift

ericnjurio and others added 2 commits January 19, 2026 12:24
Co-authored-by: adam jones <domdomegg+git@gmail.com>
@ericnjurio
ericnjurio force-pushed the fix/memory-gemini-cli-compatibility branch from 08408da to 9264308 Compare January 19, 2026 15:24
@cliffhall cliffhall added bug Something isn't working server-memory Reference implementation for the Memory MCP server - src/memory labels Apr 20, 2026
@cliffhall

Copy link
Copy Markdown
Member

@claude review

@claude

claude Bot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 2m 2s —— View job


PR Review

  • Gather context
  • Read changed files
  • Analyze the changes
  • Post review feedback

Summary

This PR adds type?: string to EntitySchema and RelationSchema and applies .passthrough() to both, aiming to fix Gemini CLI compatibility where strict JSON validators reject tool outputs that contain undeclared properties.

The fix is on the right track, but has several issues worth addressing before merging.


Issue 1: Incorrect Root Cause Analysis

The PR description states: "The memory server includes a type field in its internal JSONL storage which is subsequently returned in tool outputs."

This is not accurate based on the current code. loadGraph() (lines 79–94) explicitly strips type when reading from the JSONL file:

// In loadGraph(), type is stripped on read:
graph.entities.push({
  name: item.name,
  entityType: item.entityType,
  observations: item.observations   // no 'type' here
});

And saveGraph() (lines 103–118) re-adds type during serialization:

graph.entities.map(e => JSON.stringify({ type: "entity", name: e.name, ... }))

So tool outputs for read_graph, search_nodes, and open_nodes never include type at runtime. The more likely root cause is that Gemini CLI pre-validates the JSON Schema declaration for output schemas and rejects calls when the schema doesn't declare a type field that it expects to receive. This is a subtly different problem — and affects the approach to the fix.


Issue 2: .passthrough() is overly permissive

domdomegg already flagged this, and I agree strongly. Zod strips unknown fields by default, which is a valuable safety property — unknown input fields are silently discarded rather than propagated. .passthrough() disables that protection entirely.

Since type is now explicitly declared in the schema, .passthrough() isn't needed for the stated fix. It should be removed. Fix this →


Issue 3: z.string() is too loose for type

The type field is a discriminator with exactly two valid values: "entity" and "relation". Using z.string().optional() accepts any string, which is imprecise.

domdomegg's suggestion is correct — use z.literal():

// EntitySchema
type: z.literal("entity").optional()

// RelationSchema  
type: z.literal("relation").optional()

This is both more accurate and produces better JSON Schema for validators. Fix this →


Issue 4: Interface pollution

Adding type?: string to the Entity and Relation TypeScript interfaces (lines 50–67) is misleading. These interfaces represent the in-memory graph — the runtime data structure that never has type. The type discriminator only exists in the JSONL storage layer.

The storage format and the in-memory/communication format should stay cleanly separated. Consider leaving the interfaces unchanged and only adding type to the Zod schemas (which are used for schema declaration and wire-format validation).


Issue 5: type on input schema enables misleading input

EntitySchema and RelationSchema are used for both inputSchema and outputSchema. By adding type to the schema, clients can now pass type: "foo" when calling create_entities. The server ignores it (since saveGraph always writes the correct value), but accepting it without error is a confusing API contract.


Suggested Minimal Fix

const EntitySchema = z.object({
  type: z.literal("entity").optional(),  // discriminator only
  name: z.string().describe("The name of the entity"),
  entityType: z.string().describe("The type of the entity"),
  observations: z.array(z.string()).describe("An array of observation contents associated with the entity")
  // no .passthrough()
});

const RelationSchema = z.object({
  type: z.literal("relation").optional(),  // discriminator only
  from: z.string().describe("The name of the entity where the relation starts"),
  to: z.string().describe("The name of the entity where the relation ends"),
  relationType: z.string().describe("The type of the relation")
  // no .passthrough()
});

This would fix the schema-declaration issue (Gemini CLI sees type is declared) without opening the door to arbitrary unknown fields passing through.


Overall Assessment

The motivation is valid and the compatibility problem is real — but the fix as written is too broad. Removing .passthrough() and using z.literal() instead of z.string() would make this much more solid. The Entity/Relation interface changes could also be reverted to keep the storage/runtime separation clean.

@olaservo

olaservo commented Jun 4, 2026

Copy link
Copy Markdown
Member

Thanks for the PR! The underlying issue with the type discriminator leaking into entity output and tripping additionalProperties: false was fixed in #3213 (commit 8af50b9) by stripping type in loadGraph() instead. Since the field is no longer present on returned entities, declaring it in the schema / using .passthrough() isn't needed, and we'd prefer the output schema stay strict. Closing as superseded by the merged fix.

Drafted with assistance from Claude Code.

@olaservo olaservo closed this Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working server-memory Reference implementation for the Memory MCP server - src/memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants