From a78ca67d1db8ea733f317204b1b3e3a6d8ff0099 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Nov 2025 17:06:47 +0000 Subject: [PATCH 1/3] fix: replace Record with empty object schema for no-arg tools (#33) MCP clients with strict schema validation were rejecting tools that used `arguments: "Record"` for tools that don't take arguments. The MCP specification expects concrete schemas, and generic Record types can cause validation failures in some clients. This fix updates: - get_server_info: Changed from "Record" to {} - delete_active_file: Changed from "Record" to {} This ensures compatibility with stricter MCP clients like Gemini CLI and Q CLI that enforce proper JSON schema validation. Fixes #33 --- packages/mcp-server/src/features/local-rest-api/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mcp-server/src/features/local-rest-api/index.ts b/packages/mcp-server/src/features/local-rest-api/index.ts index 37a1424..0485776 100644 --- a/packages/mcp-server/src/features/local-rest-api/index.ts +++ b/packages/mcp-server/src/features/local-rest-api/index.ts @@ -8,7 +8,7 @@ export function registerLocalRestApiTools(tools: ToolRegistry, server: Server) { tools.register( type({ name: '"get_server_info"', - arguments: "Record", + arguments: {}, }).describe( "Returns basic details about the Obsidian Local REST API and authentication status. This is the only API request that does not require authentication.", ), @@ -134,7 +134,7 @@ export function registerLocalRestApiTools(tools: ToolRegistry, server: Server) { tools.register( type({ name: '"delete_active_file"', - arguments: "Record", + arguments: {}, }).describe("Delete the currently-active file in Obsidian."), async () => { await makeRequest(LocalRestAPI.ApiNoContentResponse, "/active/", { From b204a39decebdcb08a11d3914b1c4c7e681756d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Nov 2025 17:24:30 +0000 Subject: [PATCH 2/3] test: add tests for schema validation of no-argument tools (#33) - Test empty object schema {} accepts empty objects - Test get_server_info schema structure - Test delete_active_file schema structure - Compare Record vs {} behavior - Verify {} correctly rejects objects with properties - Test tool registration with empty arguments schema - Document why {} is more correct than Record for no-argument tools --- .../local-rest-api/schemaValidation.test.ts | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 packages/mcp-server/src/features/local-rest-api/schemaValidation.test.ts diff --git a/packages/mcp-server/src/features/local-rest-api/schemaValidation.test.ts b/packages/mcp-server/src/features/local-rest-api/schemaValidation.test.ts new file mode 100644 index 0000000..2de0d70 --- /dev/null +++ b/packages/mcp-server/src/features/local-rest-api/schemaValidation.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, test } from "bun:test"; +import { type } from "arktype"; + +/** + * Tests for schema validation of tools with no required arguments + * Issue #33: ArkType incorrectly validates Record vs {} + */ +describe("Schema validation for tools with no arguments", () => { + test("empty object schema {} accepts empty object", () => { + const schema = type({ + name: '"test_tool"', + arguments: {}, + }); + + const valid = schema({ name: "test_tool", arguments: {} }); + expect(valid instanceof type.errors).toBe(false); + }); + + test("empty object schema {} accepts object with no properties", () => { + const schema = type({ + arguments: {}, + }); + + const valid = schema({ arguments: {} }); + expect(valid instanceof type.errors).toBe(false); + }); + + test("empty object schema {} validates correctly", () => { + const schema = type({ + arguments: {}, + }); + + // Should accept empty object + const result = schema({ arguments: {} }); + if (result instanceof type.errors) { + throw new Error(`Validation failed: ${result.summary}`); + } + expect(result).toEqual({ arguments: {} }); + }); + + test("get_server_info schema structure", () => { + // Replicates the schema from get_server_info tool + const schema = type({ + name: '"get_server_info"', + arguments: {}, + }); + + const valid = schema({ + name: "get_server_info", + arguments: {}, + }); + + expect(valid instanceof type.errors).toBe(false); + }); + + test("delete_active_file schema structure", () => { + // Replicates the schema from delete_active_file tool + const schema = type({ + name: '"delete_active_file"', + arguments: {}, + }); + + const valid = schema({ + name: "delete_active_file", + arguments: {}, + }); + + expect(valid instanceof type.errors).toBe(false); + }); +}); + +describe("Comparison: Record vs {} for no-argument tools", () => { + test("empty object {} is more correct for no-argument tools", () => { + // Using {} means the arguments must be an empty object literal + const emptyObjectSchema = type({ + arguments: {}, + }); + + // This should pass + const result = emptyObjectSchema({ arguments: {} }); + expect(result instanceof type.errors).toBe(false); + }); + + test("Record would incorrectly accept any object properties", () => { + // This test documents why we changed from Record to {} + // Record means any object with string keys and unknown values + // which would incorrectly accept objects with properties for a no-argument tool + + const recordSchema = type("Record"); + + // This would incorrectly pass with Record schema + const withProperties = recordSchema({ foo: "bar", baz: 123 }); + expect(withProperties instanceof type.errors).toBe(false); + + // But {} schema correctly rejects it + const emptyObjectSchema = type({}); + const shouldReject = emptyObjectSchema({ foo: "bar" }); + + // The empty object schema should reject objects with properties + expect(shouldReject instanceof type.errors).toBe(true); + }); + + test("empty object {} only accepts objects with no properties", () => { + const schema = type({}); + + // Should accept empty object + const empty = schema({}); + expect(empty instanceof type.errors).toBe(false); + + // Should reject object with properties + const withProps = schema({ foo: "bar" }); + expect(withProps instanceof type.errors).toBe(true); + }); +}); + +describe("Tool registration with empty arguments schema", () => { + test("tool with empty arguments can be called without parameters", () => { + // Simulates the tool registration pattern used in the codebase + const toolSchema = type({ + name: '"example_tool"', + arguments: {}, + }); + + // When the tool is called with empty arguments + const callData = { + name: "example_tool", + arguments: {}, + }; + + const validated = toolSchema(callData); + expect(validated instanceof type.errors).toBe(false); + + if (!(validated instanceof type.errors)) { + expect(validated.name).toBe("example_tool"); + expect(validated.arguments).toEqual({}); + } + }); + + test("multiple tools with empty arguments schemas", () => { + const tools = [ + type({ + name: '"get_server_info"', + arguments: {}, + }), + type({ + name: '"delete_active_file"', + arguments: {}, + }), + ]; + + tools.forEach((schema) => { + const toolName = schema.infer.name; + const result = schema({ + name: toolName, + arguments: {}, + }); + expect(result instanceof type.errors).toBe(false); + }); + }); +}); From f44262c5beadf2a3d0e81c72d0b6b61cdd6f2d82 Mon Sep 17 00:00:00 2001 From: vanmarkic Date: Sat, 15 Nov 2025 19:36:05 +0100 Subject: [PATCH 3/3] fix: correct ArkType schema validation test expectations (#33) - Fix test expectations to match actual ArkType behavior - type({}) accepts objects with any properties (not strict empty) - Fix runtime access to schema.infer (TypeScript-only property) - Add test for non-object rejection - All 10 tests now pass --- .../local-rest-api/schemaValidation.test.ts | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/packages/mcp-server/src/features/local-rest-api/schemaValidation.test.ts b/packages/mcp-server/src/features/local-rest-api/schemaValidation.test.ts index 2de0d70..51155bb 100644 --- a/packages/mcp-server/src/features/local-rest-api/schemaValidation.test.ts +++ b/packages/mcp-server/src/features/local-rest-api/schemaValidation.test.ts @@ -92,24 +92,31 @@ describe("Comparison: Record vs {} for no-argument tools", () = const withProperties = recordSchema({ foo: "bar", baz: 123 }); expect(withProperties instanceof type.errors).toBe(false); - // But {} schema correctly rejects it + // Note: In ArkType, type({}) also accepts objects with additional properties + // because {} means "an object" not "an object with exactly no properties" + // For MCP tool schemas, this is actually the desired behavior for extensibility const emptyObjectSchema = type({}); - const shouldReject = emptyObjectSchema({ foo: "bar" }); + const withExtraProps = emptyObjectSchema({ foo: "bar" }); - // The empty object schema should reject objects with properties - expect(shouldReject instanceof type.errors).toBe(true); + // Both patterns accept objects (the issue was that Record + // was not being recognized properly by some MCP clients) + expect(withExtraProps instanceof type.errors).toBe(false); }); - test("empty object {} only accepts objects with no properties", () => { + test("empty object {} accepts objects regardless of properties", () => { const schema = type({}); - // Should accept empty object + // Accepts empty object const empty = schema({}); expect(empty instanceof type.errors).toBe(false); - // Should reject object with properties + // Also accepts object with properties (in ArkType, {} means "object type") const withProps = schema({ foo: "bar" }); - expect(withProps instanceof type.errors).toBe(true); + expect(withProps instanceof type.errors).toBe(false); + + // But rejects non-objects + const notObject = schema("not an object"); + expect(notObject instanceof type.errors).toBe(true); }); }); @@ -138,18 +145,23 @@ describe("Tool registration with empty arguments schema", () => { test("multiple tools with empty arguments schemas", () => { const tools = [ - type({ - name: '"get_server_info"', - arguments: {}, - }), - type({ - name: '"delete_active_file"', - arguments: {}, - }), + { + schema: type({ + name: '"get_server_info"', + arguments: {}, + }), + name: "get_server_info", + }, + { + schema: type({ + name: '"delete_active_file"', + arguments: {}, + }), + name: "delete_active_file", + }, ]; - tools.forEach((schema) => { - const toolName = schema.infer.name; + tools.forEach(({ schema, name: toolName }) => { const result = schema({ name: toolName, arguments: {},