diff --git a/CHANGELOG.md b/CHANGELOG.md index 70b9699..8cd9edc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.12.2] - 2026-04-07 + +### Fixed + +- **`full` tool profile breaks OpenAI-compatible providers** (#33) + - `context_delegate.input.insights` array property was missing required `items` declaration + - Stricter providers rejected the schema with `invalid_function_parameters` + - Added `items: { type: 'object', properties: {...} }` to the `insights` property with proper `patterns`, `relationships`, `trends`, and `themes` fields + - Added `properties: {}` to `context_link.metadata` bare object schema for strict validator compatibility + - Added regression test that validates all array properties across all tool schemas have `items` declared + - Added comprehensive E2E test suite that spawns the actual MCP server and validates tool schemas, tool calls, and error handling over stdio + ## [0.12.1] - 2026-03-24 ### Fixed @@ -507,7 +519,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Security**: Security updates - **Technical**: Internal improvements -[Unreleased]: https://github.com/mkreyman/mcp-memory-keeper/compare/v0.12.1...HEAD +[Unreleased]: https://github.com/mkreyman/mcp-memory-keeper/compare/v0.12.2...HEAD +[0.12.2]: https://github.com/mkreyman/mcp-memory-keeper/compare/v0.12.1...v0.12.2 [0.12.1]: https://github.com/mkreyman/mcp-memory-keeper/compare/v0.12.0...v0.12.1 [0.12.0]: https://github.com/mkreyman/mcp-memory-keeper/compare/v0.11.0...v0.12.0 [0.11.0]: https://github.com/mkreyman/mcp-memory-keeper/compare/v0.10.2...v0.11.0 diff --git a/package-lock.json b/package-lock.json index c55075b..fbd524c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "mcp-memory-keeper", - "version": "0.12.1", + "version": "0.12.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "mcp-memory-keeper", - "version": "0.12.1", + "version": "0.12.2", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.12.3", diff --git a/package.json b/package.json index 53d684a..1934441 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "mcp-memory-keeper", - "version": "0.12.1", + "version": "0.12.2", "description": "MCP server for persistent context management in AI coding assistants", "main": "dist/index.js", "bin": { diff --git a/src/__tests__/e2e/server-e2e.test.ts b/src/__tests__/e2e/server-e2e.test.ts new file mode 100644 index 0000000..8f4fa06 --- /dev/null +++ b/src/__tests__/e2e/server-e2e.test.ts @@ -0,0 +1,388 @@ +import { describe, it, expect, beforeAll, afterAll } from '@jest/globals'; +import { spawn, ChildProcess } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +/** + * End-to-end tests for the MCP Memory Keeper server. + * + * These tests spawn the actual server process over stdio, send real MCP + * protocol messages, and validate the responses. Unlike integration tests + * that instantiate internal classes directly, these exercise the full stack: + * transport → protocol → handler → database → response. + * + * @see https://github.com/mkreyman/mcp-memory-keeper/issues/33 + */ + +// Valid JSON Schema types per the spec +const VALID_JSON_SCHEMA_TYPES = new Set([ + 'string', + 'number', + 'integer', + 'boolean', + 'array', + 'object', + 'null', +]); + +let serverProcess: ChildProcess | null = null; +let tempDir: string; +let msgId = 0; +let outputBuffer = ''; + +/** Send a JSON-RPC message to the server and wait for a response with the matching id. */ +function sendRequest( + method: string, + params: Record = {}, + timeoutMs = 5000 +): Promise { + return new Promise((resolve, reject) => { + const id = ++msgId; + const timeout = setTimeout(() => { + reject(new Error(`Timeout waiting for response to ${method} (id=${id})`)); + }, timeoutMs); + + const onData = (data: Buffer) => { + outputBuffer += data.toString(); + const lines = outputBuffer.split('\n'); + // Keep the last incomplete line in the buffer + outputBuffer = lines.pop() || ''; + + for (const line of lines) { + if (!line.trim()) continue; + try { + const msg = JSON.parse(line); + if (msg.id === id) { + clearTimeout(timeout); + serverProcess?.stdout?.removeListener('data', onData); + resolve(msg); + } + } catch { + // Not JSON, skip + } + } + }; + + serverProcess?.stdout?.on('data', onData); + serverProcess?.stdin?.write(JSON.stringify({ jsonrpc: '2.0', method, params, id }) + '\n'); + }); +} + +/** + * Recursively validate a JSON Schema property definition. + * Returns an array of human-readable violation strings. + */ +function validateSchemaProperty( + prop: Record, + path: string, + toolName: string +): string[] { + const violations: string[] = []; + + // Every property should have a type + if (!prop.type) { + violations.push(`${toolName}: ${path} — missing 'type'`); + return violations; // Can't validate further without type + } + + // Type must be a valid JSON Schema type + if (!VALID_JSON_SCHEMA_TYPES.has(prop.type)) { + violations.push(`${toolName}: ${path} — invalid type '${prop.type}'`); + } + + // Arrays must have items + if (prop.type === 'array' && !prop.items) { + violations.push(`${toolName}: ${path} — type 'array' missing 'items'`); + } + + // If enum is present, values should match the declared type + if (prop.enum && prop.type) { + for (const val of prop.enum) { + if (prop.type === 'string' && typeof val !== 'string') { + violations.push(`${toolName}: ${path} — enum value '${val}' is not a string`); + } + if (prop.type === 'number' && typeof val !== 'number') { + violations.push(`${toolName}: ${path} — enum value '${val}' is not a number`); + } + } + } + + // Required must reference existing properties + if (prop.type === 'object' && prop.required && prop.properties) { + for (const req of prop.required) { + if (!(req in prop.properties)) { + violations.push(`${toolName}: ${path} — required field '${req}' not found in properties`); + } + } + } + + // Recurse into object properties + if (prop.type === 'object' && prop.properties) { + for (const [key, value] of Object.entries(prop.properties)) { + violations.push( + ...validateSchemaProperty(value as Record, `${path}.${key}`, toolName) + ); + } + } + + // Recurse into array items + if (prop.type === 'array' && prop.items && typeof prop.items === 'object') { + violations.push( + ...validateSchemaProperty(prop.items as Record, `${path}.items`, toolName) + ); + } + + return violations; +} + +describe('MCP Server E2E Tests', () => { + beforeAll(async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mcp-e2e-')); + + serverProcess = spawn('node', [path.join(__dirname, '../../../dist/index.js')], { + env: { ...process.env, DATA_DIR: tempDir }, + stdio: ['pipe', 'pipe', 'pipe'], + }); + + if ((global as any).testProcesses) { + (global as any).testProcesses.push(serverProcess); + } + + // Initialize the MCP session + const initResponse = await sendRequest('initialize', { + protocolVersion: '2024-11-05', + capabilities: {}, + clientInfo: { name: 'e2e-test', version: '1.0.0' }, + }); + + expect(initResponse.result).toHaveProperty('protocolVersion'); + + // Send initialized notification (no response expected) + serverProcess?.stdin?.write( + JSON.stringify({ jsonrpc: '2.0', method: 'notifications/initialized' }) + '\n' + ); + + // Brief pause for server to process the notification + await new Promise(resolve => setTimeout(resolve, 200)); + }, 10000); + + afterAll(async () => { + if (serverProcess && !serverProcess.killed) { + serverProcess.kill('SIGTERM'); + await new Promise(resolve => { + const timeout = setTimeout(() => { + serverProcess?.kill('SIGKILL'); + resolve(); + }, 3000); + serverProcess?.on('exit', () => { + clearTimeout(timeout); + resolve(); + }); + }); + serverProcess?.removeAllListeners(); + } + serverProcess = null; + + try { + fs.rmSync(tempDir, { recursive: true, force: true }); + } catch { + // ignore cleanup errors + } + }); + + // ─── ListTools Schema Validation ──────────────────────────────────── + + describe('tools/list — schema validation', () => { + let tools: any[]; + + beforeAll(async () => { + const response = await sendRequest('tools/list'); + expect(response.result).toHaveProperty('tools'); + tools = response.result.tools; + }); + + it('should return a non-empty list of tools', () => { + expect(tools.length).toBeGreaterThan(0); + }); + + it('every tool should have name, description, and inputSchema', () => { + for (const tool of tools) { + expect(tool).toHaveProperty('name'); + expect(tool).toHaveProperty('description'); + expect(tool).toHaveProperty('inputSchema'); + expect(typeof tool.name).toBe('string'); + expect(typeof tool.description).toBe('string'); + expect(tool.name.length).toBeGreaterThan(0); + expect(tool.description.length).toBeGreaterThan(0); + } + }); + + it('every tool name should follow the context_ convention', () => { + for (const tool of tools) { + expect(tool.name).toMatch(/^context_[a-z_]+$/); + } + }); + + it('every inputSchema should be type object with properties', () => { + for (const tool of tools) { + expect(tool.inputSchema.type).toBe('object'); + expect(tool.inputSchema).toHaveProperty('properties'); + expect(typeof tool.inputSchema.properties).toBe('object'); + } + }); + + it('no tool names should be duplicated', () => { + const names = tools.map((t: any) => t.name); + expect(new Set(names).size).toBe(names.length); + }); + + it('every array property should have items declared (issue #33)', () => { + const violations: string[] = []; + for (const tool of tools) { + violations.push( + ...validateSchemaProperty(tool.inputSchema, 'inputSchema', tool.name).filter(v => + v.includes("missing 'items'") + ) + ); + } + expect(violations).toHaveLength(0); + }); + + it('every property type should be a valid JSON Schema type', () => { + const violations: string[] = []; + for (const tool of tools) { + violations.push( + ...validateSchemaProperty(tool.inputSchema, 'inputSchema', tool.name).filter(v => + v.includes('invalid type') + ) + ); + } + expect(violations).toHaveLength(0); + }); + + it('every required field should reference an existing property', () => { + const violations: string[] = []; + for (const tool of tools) { + violations.push( + ...validateSchemaProperty(tool.inputSchema, 'inputSchema', tool.name).filter(v => + v.includes('not found in properties') + ) + ); + } + expect(violations).toHaveLength(0); + }); + + it('enum values should match their declared type', () => { + const violations: string[] = []; + for (const tool of tools) { + violations.push( + ...validateSchemaProperty(tool.inputSchema, 'inputSchema', tool.name).filter(v => + v.includes('enum value') + ) + ); + } + expect(violations).toHaveLength(0); + }); + + it('full schema validation should find zero violations across all tools', () => { + const allViolations: string[] = []; + for (const tool of tools) { + allViolations.push(...validateSchemaProperty(tool.inputSchema, 'inputSchema', tool.name)); + } + if (allViolations.length > 0) { + throw new Error( + `Found ${allViolations.length} schema violation(s):\n${allViolations.map(v => ` ${v}`).join('\n')}` + ); + } + }); + }); + + // ─── Tool Call Smoke Tests ────────────────────────────────────────── + + describe('tools/call — smoke tests', () => { + it('should save and retrieve a context item', async () => { + // Save + const saveResponse = await sendRequest('tools/call', { + name: 'context_save', + arguments: { + key: 'e2e_test_key', + value: 'e2e_test_value', + category: 'note', + }, + }); + + expect(saveResponse.result).toHaveProperty('content'); + const saveText = saveResponse.result.content[0].text; + expect(saveText).toContain('e2e_test_key'); + + // Retrieve + const getResponse = await sendRequest('tools/call', { + name: 'context_get', + arguments: { key: 'e2e_test_key' }, + }); + + expect(getResponse.result).toHaveProperty('content'); + const getResult = JSON.parse(getResponse.result.content[0].text); + expect(getResult.items).toBeDefined(); + expect(getResult.items.length).toBeGreaterThan(0); + expect(getResult.items[0].value).toBe('e2e_test_value'); + }); + + it('should search for saved items', async () => { + const response = await sendRequest('tools/call', { + name: 'context_search', + arguments: { query: 'e2e_test' }, + }); + + expect(response.result).toHaveProperty('content'); + const text = response.result.content[0].text; + expect(text).toContain('e2e_test'); + }); + + it('should return status', async () => { + const response = await sendRequest('tools/call', { + name: 'context_status', + arguments: {}, + }); + + expect(response.result).toHaveProperty('content'); + const text = response.result.content[0].text; + // Status response contains session info regardless of format + expect(text.length).toBeGreaterThan(0); + expect(text).toMatch(/session|item/i); + }); + + it('should reject unknown tools with an error', async () => { + const response = await sendRequest('tools/call', { + name: 'nonexistent_tool', + arguments: {}, + }); + + // MCP SDK wraps handler errors + expect(response.error || response.result?.isError).toBeTruthy(); + }); + + it('should handle missing required arguments gracefully', async () => { + const response = await sendRequest('tools/call', { + name: 'context_save', + arguments: {}, + }); + + expect(response.result).toHaveProperty('content'); + const text = response.result.content[0].text; + // Should return an error message, not crash + expect(text.toLowerCase()).toMatch(/error|required|key/i); + }); + }); + + // ─── Tool Profile Filtering ──────────────────────────────────────── + + describe('tool profile filtering', () => { + it('default profile should expose all tools', async () => { + const response = await sendRequest('tools/list'); + // Default is "full" profile with 38 tools + expect(response.result.tools.length).toBe(38); + }); + }); +}); diff --git a/src/__tests__/integration/issue33-array-items-schema.test.ts b/src/__tests__/integration/issue33-array-items-schema.test.ts new file mode 100644 index 0000000..8ce7f26 --- /dev/null +++ b/src/__tests__/integration/issue33-array-items-schema.test.ts @@ -0,0 +1,151 @@ +import { describe, it, expect } from '@jest/globals'; +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Issue #33: full tool profile exposes a schema that breaks some OpenAI-compatible providers + * + * Some providers reject tool schemas where an array property lacks an `items` declaration. + * Per JSON Schema spec, `items` is required for `type: 'array'` to fully describe the schema. + * + * This test scans src/index.ts and verifies that every property with `type: 'array'` + * has an `items` declaration within the same schema block. + * + * @see https://github.com/mkreyman/mcp-memory-keeper/issues/33 + */ + +/** + * Scan the source for tool definitions and find array properties missing `items`. + * Skips block comments to avoid false positives from commented-out schemas. + * Line numbers refer to the original source file for accurate debugging. + */ +function findArrayPropertiesMissingItems( + src: string +): Array<{ tool: string; property: string; line: number }> { + const violations: Array<{ tool: string; property: string; line: number }> = []; + const lines = src.split('\n'); + + let currentTool = ''; + let inBlockComment = false; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + + // Track block comment state + if (inBlockComment) { + if (line.includes('*/')) { + inBlockComment = false; + } + continue; + } + if (line.includes('/*')) { + inBlockComment = true; + if (line.includes('*/')) { + inBlockComment = false; + } + continue; + } + + // Track which tool we're inside + const toolMatch = line.match(/name:\s*'(context_[a-z_]+)'/); + if (toolMatch) { + currentTool = toolMatch[1]; + } + + // Find array type declarations + if (!line.match(/type:\s*'array'/)) continue; + if (!currentTool) continue; + + // Find the property name (check current line first for single-line declarations) + let propertyName = '(unknown)'; + const currentLinePropMatch = line.match(/(\w+)\s*:\s*\{/); + if (currentLinePropMatch) { + propertyName = currentLinePropMatch[1]; + } else { + for (let j = i - 1; j >= Math.max(0, i - 5); j--) { + const propMatch = lines[j].match(/(\w+)\s*:\s*\{/); + if (propMatch) { + propertyName = propMatch[1]; + break; + } + } + } + + // Check current line first (handles single-line array declarations) + let foundItems = line.includes('items'); + let depth = 0; + + for (let j = i + 1; !foundItems && j < Math.min(lines.length, i + 50); j++) { + const fwdLine = lines[j]; + + for (const ch of fwdLine) { + if (ch === '{') depth++; + if (ch === '}') depth--; + } + + if (fwdLine.match(/^\s*items\s*:/)) { + foundItems = true; + break; + } + + if (depth < 0) break; + } + + if (!foundItems) { + violations.push({ + tool: currentTool, + property: propertyName, + line: i + 1, + }); + } + } + + return violations; +} + +describe('Issue #33: Array properties must declare items', () => { + const indexPath = path.join(__dirname, '..', '..', 'index.ts'); + const src = fs.readFileSync(indexPath, 'utf-8'); + + it('should find tool definitions in source', () => { + const toolNames = src.match(/name:\s*'context_[a-z_]+'/g); + expect(toolNames).not.toBeNull(); + expect(toolNames!.length).toBeGreaterThan(0); + }); + + it('every array property in every tool schema must have an items declaration', () => { + const violations = findArrayPropertiesMissingItems(src); + + expect(violations).toHaveLength(0); + }); + + it('context_delegate.input.insights specifically must have items', () => { + const lines = src.split('\n'); + let inDelegate = false; + let insightsLine = -1; + + for (let i = 0; i < lines.length; i++) { + if (lines[i].match(/name:\s*'context_delegate'/)) { + inDelegate = true; + } + if ( + inDelegate && + i > 0 && + lines[i].match(/name:\s*'context_/) && + !lines[i].match(/context_delegate/) + ) { + break; + } + if (inDelegate && lines[i].match(/insights\s*:\s*\{/)) { + insightsLine = i; + break; + } + } + + expect(insightsLine).toBeGreaterThan(-1); + + const insightsBlock = lines.slice(insightsLine, insightsLine + 5).join('\n'); + expect(insightsBlock).toContain("type: 'array'"); + expect(insightsBlock).toMatch(/items\s*:/); + }); +}); diff --git a/src/index.ts b/src/index.ts index 50fa443..e61e577 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4501,6 +4501,15 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { }, insights: { type: 'array', + items: { + type: 'object', + properties: { + patterns: { type: 'object', properties: {} }, + relationships: { type: 'object', properties: {} }, + trends: { type: 'object', properties: {} }, + themes: { type: 'array', items: { type: 'string' } }, + }, + }, description: 'For merge synthesis: array of insights to merge', }, }, @@ -5040,6 +5049,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { }, metadata: { type: 'object', + properties: {}, description: 'Optional metadata for the relationship', }, },