fix(api): reject undeclared and off-enum tool arguments (policy R4)

docs/api-error-policy.md

@@ -49,6 +49,10 @@ Every mutating success enumerates per-item outcomes
 An item that cannot be satisfied errors (R1) unless the caller's intent is
 provably satisfied — duplicates are "already there", deleting an absent
 thing is "already absent". Those cases succeed **and are reported** (R3).
+This covers arguments too: a key the tool's `inputSchema` does not declare,
+or a value outside a declared `enum`, is `VALIDATION_FAILED` (every offender
+listed, the tool does not run) — never ignored. A misnamed cursor key names
+the cursor parameter(s) the tool actually takes.
 
 **R5 — Pagination integrity.**
 Cursors identify the result set they came from (a fingerprint now; a store

server.ts

@@ -403,6 +403,77 @@ function paginateGraph(graph: KnowledgeGraph, entityCursor: number | string = 0,
   };
 }
 
+/** The slice of a tool's JSON-Schema `inputSchema` that argument validation reads. */
+interface ToolArgSchema {
+  // `| undefined`: the inferred union of all tool schemas pads each one with
+  // the other tools' property names as optional-undefined keys.
+  properties?: Record<string, object | undefined>;
+}
+
+/** One rejected argument, reported in the VALIDATION_FAILED details list. */
+interface ArgViolation {
+  argument: string;
+  code: 'VALIDATION_FAILED';
+  message: string;
+  allowed: unknown[];
+}
+
+/**
+ * Policy R4 (docs/api-error-policy.md) applied to arguments: an argument the
+ * tool does not declare, or a value outside a declared enum, is an error — not
+ * something to drop. Before this check the server ignored both silently, so a
+ * misnamed cursor (open_nodes `cursor` instead of `relationCursor`) re-served
+ * page 1 with the same nextCursor, and an off-enum `direction` ran as some
+ * other direction.
+ *
+ * Validate-first (R2): every offending argument is reported in one error and
+ * the tool does not run.
+ */
+export function validateToolArgs(tool: string, schema: ToolArgSchema, args: Record<string, unknown>): void {
+  const properties = schema.properties ?? {};
+  const declared = Object.keys(properties);
+  const violations: ArgViolation[] = [];
+
+  for (const [key, value] of Object.entries(args)) {
+    const spec = Object.prototype.hasOwnProperty.call(properties, key) ? properties[key] : undefined;
+    if (spec === undefined) {
+      violations.push({ argument: key, code: 'VALIDATION_FAILED', message: unknownArgumentMessage(tool, key, declared), allowed: declared });
+      continue;
+    }
+    const allowedValues = (spec as { enum?: unknown }).enum;
+    if (Array.isArray(allowedValues) && !allowedValues.includes(value)) {
+      violations.push({
+        argument: key,
+        code: 'VALIDATION_FAILED',
+        message: `${JSON.stringify(value)} is not a valid ${key} for ${tool}; expected one of ${allowedValues.map(v => JSON.stringify(v)).join(', ')}.`,
+        allowed: [...allowedValues],
+      });
+    }
+  }
+
+  if (violations.length > 0) {
+    throw new ToolError(
+      'VALIDATION_FAILED',
+      `${violations.length} argument(s) rejected for ${tool}; the tool did not run (details below).`,
+      violations,
+    );
+  }
+}
+
+/**
+ * Cursor parameter names differ between tools (`cursor` on single-list tools,
+ * `entityCursor` / `relationCursor` on graph-shaped ones), which is exactly
+ * the slip that hid open_nodes pages. Name the right key when that happens.
+ */
+function unknownArgumentMessage(tool: string, key: string, declared: string[]): string {
+  const base = `${JSON.stringify(key)} is not a parameter of ${tool}.`;
+  const cursorParams = declared.filter(k => /cursor$/i.test(k));
+  if (/cursor$/i.test(key) && cursorParams.length > 0) {
+    return `${base} Its cursor parameter(s): ${cursorParams.join(', ')} — pass each nextCursor back under the key of the section it came from.`;
+  }
+  return `${base} Declared parameters: ${declared.join(', ')}.`;
+}
+
 // The KnowledgeGraphManager class contains all operations to interact with the knowledge graph
 export class KnowledgeGraphManager {
   private db: Store;
@@ -1521,9 +1592,9 @@ export function createServer(memoryFilePath?: string): Server {
     knowledgeGraphManager.close();
   };
 
-server.setRequestHandler(ListToolsRequestSchema, async () => {
-  return {
-    tools: [
+  // The advertised tool list is also the argument contract: dispatch validates
+  // every call against these same inputSchemas (validateToolArgs, policy R4).
+  const toolDefinitions = [
       {
         name: "create_entities",
         description: "Create multiple new entities in the knowledge graph",
@@ -1874,9 +1945,9 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p
           required: ["filePath"],
         },
       },
-    ],
-  };
-});
+  ];
+
+  server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: toolDefinitions }));
 
   /**
    * Dispatch a single tool call. Extracted from the request handler so the
@@ -1889,6 +1960,8 @@ The file MUST be plaintext (.txt, .tex, .md, source code, etc.). For PDFs, use p
    * remains a JSON-RPC protocol error and is reserved for server faults.
    */
   async function dispatch(name: string, args: Record<string, unknown>): Promise<ToolDispatchResult> {
+    const definition = toolDefinitions.find(t => t.name === name);
+    if (definition) validateToolArgs(name, definition.inputSchema, args);
     switch (name) {
       case "create_entities": {
         const result = await knowledgeGraphManager.createEntities(args.entities as Entity[]);

tests/arg-validation.test.ts

@@ -0,0 +1,128 @@
+import { describe, it, expect, beforeEach, afterEach } from '@jest/globals';
+import { promises as fs } from 'fs';
+import path from 'path';
+import os from 'os';
+import { createServer, type Relation } from '../server.js';
+import { createTestClient, callTool, callToolRaw, parseToolError, type PaginatedGraph } from './test-utils.js';
+
+interface ArgViolation {
+  argument: string;
+  code: string;
+  message: string;
+  allowed: unknown[];
+}
+
+// Policy R4 applied to arguments: undeclared keys and off-enum values are
+// rejected, never dropped. Regression: open_nodes called with `cursor` (the
+// get_neighbors key) instead of `relationCursor` silently re-served page 1
+// with an identical nextCursor, making every edge past page 1 unreachable.
+describe('Tool argument validation (R4: no silently dropped arguments)', () => {
+  let testDir: string;
+  let client: Awaited<ReturnType<typeof createTestClient>>['client'];
+  let cleanup: () => Promise<void>;
+
+  beforeEach(async () => {
+    testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-memory-argval-'));
+    const server = createServer(path.join(testDir, 'test-memory.json'));
+    const result = await createTestClient(server);
+    client = result.client;
+    cleanup = result.cleanup;
+  });
+
+  afterEach(async () => {
+    await cleanup();
+    try {
+      await fs.rm(testDir, { recursive: true });
+    } catch {
+      // Ignore cleanup errors
+    }
+  });
+
+  async function seedHub(): Promise<void> {
+    await callTool(client, 'create_entities', {
+      entities: [{ name: 'Hub', entityType: 'Test', observations: [] }],
+    });
+  }
+
+  it('rejects open_nodes `cursor` and names the cursor keys it does take', async () => {
+    await seedHub();
+    const raw = await callToolRaw(client, 'open_nodes', {
+      names: ['Hub'], direction: 'backward', cursor: '31:950827ed',
+    });
+    expect(raw.isError).toBe(true);
+    const err = parseToolError(raw);
+    expect(err.code).toBe('VALIDATION_FAILED');
+    const details = err.details as ArgViolation[];
+    expect(details).toHaveLength(1);
+    expect(details[0].argument).toBe('cursor');
+    expect(details[0].message).toContain('entityCursor');
+    expect(details[0].message).toContain('relationCursor');
+  });
+
+  it('rejects get_neighbors `relationCursor` and names `cursor`', async () => {
+    await seedHub();
+    const raw = await callToolRaw(client, 'get_neighbors', { entityName: 'Hub', relationCursor: '0:00000000' });
+    const err = parseToolError(raw);
+    expect(err.code).toBe('VALIDATION_FAILED');
+    const details = err.details as ArgViolation[];
+    expect(details[0].argument).toBe('relationCursor');
+    expect(details[0].message).toMatch(/cursor parameter\(s\): cursor\b/);
+  });
+
+  it('rejects an off-enum direction instead of running it as something else', async () => {
+    await seedHub();
+    const raw = await callToolRaw(client, 'open_nodes', { names: ['Hub'], direction: 'both' });
+    const err = parseToolError(raw);
+    expect(err.code).toBe('VALIDATION_FAILED');
+    const details = err.details as ArgViolation[];
+    expect(details).toHaveLength(1);
+    expect(details[0].argument).toBe('direction');
+    expect(details[0].allowed).toEqual(['forward', 'backward', 'any']);
+  });
+
+  it('reports every offending argument in one error (validate-first)', async () => {
+    await seedHub();
+    const raw = await callToolRaw(client, 'open_nodes', {
+      names: ['Hub'], direction: 'both', cursor: '31:950827ed',
+    });
+    const details = parseToolError(raw).details as ArgViolation[];
+    expect(details.map(d => d.argument).sort()).toEqual(['cursor', 'direction']);
+  });
+
+  it('a write with an undeclared argument changes nothing', async () => {
+    const raw = await callToolRaw(client, 'create_entities', {
+      entities: [{ name: 'NeverWritten', entityType: 'Test', observations: [] }],
+      dryRun: true,
+    });
+    expect(parseToolError(raw).code).toBe('VALIDATION_FAILED');
+    const open = await callToolRaw(client, 'open_nodes', { names: ['NeverWritten'] });
+    expect(parseToolError(open).code).toBe('ENTITY_NOT_FOUND');
+  });
+
+  it('drains a multi-page backward relation list via relationCursor', async () => {
+    await seedHub();
+    const sources = Array.from({ length: 120 }, (_, i) => `Src${String(i).padStart(3, '0')}`);
+    await callTool(client, 'create_entities', {
+      entities: sources.map(name => ({ name, entityType: 'Test', observations: [] })),
+    });
+    await callTool(client, 'create_relations', {
+      relations: sources.map(from => ({ from, to: 'Hub', relationType: 'POINTS_AT' })),
+    });
+
+    const seen: Relation[] = [];
+    let relationCursor: string | number | null = 0;
+    let pages = 0;
+    while (relationCursor !== null && pages < 50) {
+      const page = await callTool(client, 'open_nodes', {
+        names: ['Hub'], direction: 'backward', relationCursor,
+      }) as PaginatedGraph;
+      seen.push(...page.relations.items);
+      relationCursor = page.relations.nextCursor;
+      pages++;
+    }
+
+    expect(pages).toBeGreaterThan(1);
+    expect(new Set(seen.map(r => r.from)).size).toBe(120);
+    expect(seen).toHaveLength(120);
+  });
+});