fix(memory): add 'type' to schemas and use passthrough for Gemini CLI… - #3149
fix(memory): add 'type' to schemas and use passthrough for Gemini CLI…#3149ericnjurio wants to merge 2 commits into
Conversation
Changes SummaryThis 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
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
Full review in progress... | Powered by diffray |
|
|
||
| // We are storing our memory using entities, relations, and observations in a graph structure | ||
| export interface Entity { | ||
| type?: string; |
There was a problem hiding this comment.
should we just change this to the const "entity" and relation to "relation"?
| 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(); |
There was a problem hiding this comment.
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
Co-authored-by: adam jones <domdomegg+git@gmail.com>
08408da to
9264308
Compare
|
@claude review |
|
Claude finished @cliffhall's task in 2m 2s —— View job PR Review
SummaryThis PR adds The fix is on the right track, but has several issues worth addressing before merging. Issue 1: Incorrect Root Cause AnalysisThe 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. // In loadGraph(), type is stripped on read:
graph.entities.push({
name: item.name,
entityType: item.entityType,
observations: item.observations // no 'type' here
});And graph.entities.map(e => JSON.stringify({ type: "entity", name: e.name, ... }))So tool outputs for Issue 2:
|
|
Thanks for the PR! The underlying issue with the Drafted with assistance from Claude Code. |
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:
Server Details
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:
Breaking Changes
Types of changes
Checklist
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.