Skip to content

Commit 529123a

Browse files
committed
Add server tool-schema-validation.test.ts
Adds tests to generically avoid regressions due to invalid schema for any MCP tool.
1 parent eb6e27f commit 529123a

1 file changed

Lines changed: 211 additions & 0 deletions

File tree

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
/**
2+
* Generic JSON Schema validation for ALL MCP server tools.
3+
*
4+
* Regression test for the class of bug where Zod types (e.g. `z.tuple()`)
5+
* serialize to JSON Schema values that strict validators (GitHub Copilot Chat,
6+
* OpenAI) reject with HTTP 400. Specifically, every property's JSON Schema
7+
* value MUST be an object or boolean — never a bare array.
8+
*
9+
* This test registers every tool category and validates the serialized
10+
* JSON Schema output so that no future tool can introduce a similar problem.
11+
*
12+
* @see https://github.com/advanced-security/codeql-development-mcp-server/pull/263
13+
*/
14+
15+
import { describe, expect, it, vi } from 'vitest';
16+
17+
// Mock the logger to silence output during tests
18+
vi.mock('../../../src/utils/logger', () => ({
19+
logger: {
20+
debug: vi.fn(),
21+
info: vi.fn(),
22+
warn: vi.fn(),
23+
error: vi.fn(),
24+
},
25+
}));
26+
27+
// Mock session-data-manager to prevent file system side effects
28+
vi.mock('../../../src/lib/session-data-manager', () => ({
29+
sessionDataManager: {
30+
getStore: vi.fn().mockReturnValue({
31+
getCacheEntryMeta: vi.fn(),
32+
listCacheEntries: vi.fn().mockReturnValue([]),
33+
getCacheEntryContent: vi.fn(),
34+
clearCacheEntries: vi.fn(),
35+
}),
36+
getConfig: vi.fn().mockReturnValue({
37+
storageLocation: '/tmp/test',
38+
autoTrackSessions: false,
39+
retentionDays: 90,
40+
includeCallParameters: false,
41+
includeCallResults: false,
42+
maxActiveSessionsPerQuery: 3,
43+
scoringFrequency: 'per_call',
44+
archiveCompletedSessions: false,
45+
enableAnnotationTools: true,
46+
enableRecommendations: false,
47+
enableMonitoringTools: true,
48+
}),
49+
initialize: vi.fn(),
50+
},
51+
}));
52+
53+
// Mock CLI executor to prevent actual CodeQL binary resolution
54+
vi.mock('../../../src/lib/cli-executor', () => ({
55+
resolveCodeQLBinary: vi.fn().mockReturnValue('codeql'),
56+
executeCodeQLCommand: vi.fn().mockResolvedValue({ stdout: '', stderr: '' }),
57+
}));
58+
59+
// Mock server-manager to prevent JVM warm-up
60+
vi.mock('../../../src/lib/server-manager', () => ({
61+
initServerManager: vi.fn().mockReturnValue({
62+
warmUpLanguageServer: vi.fn(),
63+
warmUpCLIServer: vi.fn(),
64+
}),
65+
shutdownServerManager: vi.fn(),
66+
getServerManager: vi.fn().mockReturnValue(null),
67+
}));
68+
69+
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
70+
import { registerAnnotationTools } from '../../../src/tools/annotation-tools';
71+
import { registerAuditTools } from '../../../src/tools/audit-tools';
72+
import { registerCacheTools } from '../../../src/tools/cache-tools';
73+
import { registerCodeQLTools } from '../../../src/tools/codeql-tools';
74+
import { registerLSPTools } from '../../../src/tools/lsp';
75+
import { registerMonitoringTools } from '../../../src/tools/monitoring-tools';
76+
import { registerSarifTools } from '../../../src/tools/sarif-tools';
77+
78+
/**
79+
* Helper: register all tool categories onto a single McpServer instance,
80+
* mirroring the registration order in `codeql-development-mcp-server.ts`.
81+
*/
82+
function registerAllTools(server: McpServer): void {
83+
registerCodeQLTools(server);
84+
registerLSPTools(server);
85+
registerMonitoringTools(server);
86+
registerAnnotationTools(server);
87+
registerAuditTools(server);
88+
registerCacheTools(server);
89+
registerSarifTools(server);
90+
}
91+
92+
describe('Tool JSON Schema Validation (all tools)', () => {
93+
it('every tool inputSchema property must be an object or boolean — never a bare array', async () => {
94+
const server = new McpServer({ name: 'schema-test', version: '0.0.0' });
95+
registerAllTools(server);
96+
97+
// Access registered tools via internal SDK handle — same code path as
98+
// the live `tools/list` JSON-RPC handler.
99+
const registered = (server as any)._registeredTools as
100+
Record<string, { inputSchema?: unknown }>;
101+
102+
expect(Object.keys(registered).length).toBeGreaterThan(0);
103+
104+
const { toJsonSchemaCompat } = await import(
105+
'@modelcontextprotocol/sdk/server/zod-json-schema-compat.js'
106+
);
107+
108+
const offending: string[] = [];
109+
110+
for (const [toolName, def] of Object.entries(registered)) {
111+
if (!def.inputSchema) continue;
112+
113+
const jsonSchema = toJsonSchemaCompat(def.inputSchema as never) as {
114+
type?: string;
115+
properties?: Record<string, unknown>;
116+
};
117+
118+
// Top-level schema must be an object, not an array.
119+
if (Array.isArray(jsonSchema)) {
120+
offending.push(`${toolName} (top-level schema is an array)`);
121+
continue;
122+
}
123+
124+
// Walk every property's schema value.
125+
for (const [propName, propSchema] of Object.entries(jsonSchema.properties ?? {})) {
126+
const isValid =
127+
typeof propSchema === 'boolean' ||
128+
(typeof propSchema === 'object' && propSchema !== null && !Array.isArray(propSchema));
129+
if (!isValid) {
130+
offending.push(`${toolName}.${propName} = ${JSON.stringify(propSchema)}`);
131+
}
132+
133+
// Recurse one level into nested object properties (catches nested
134+
// tuples like `z.object({ nested: z.tuple([...]) })`).
135+
if (
136+
typeof propSchema === 'object' &&
137+
propSchema !== null &&
138+
!Array.isArray(propSchema)
139+
) {
140+
const nested = (propSchema as { properties?: Record<string, unknown> }).properties;
141+
if (nested) {
142+
for (const [nestedName, nestedSchema] of Object.entries(nested)) {
143+
const isNestedValid =
144+
typeof nestedSchema === 'boolean' ||
145+
(typeof nestedSchema === 'object' && nestedSchema !== null && !Array.isArray(nestedSchema));
146+
if (!isNestedValid) {
147+
offending.push(
148+
`${toolName}.${propName}.${nestedName} = ${JSON.stringify(nestedSchema)}`,
149+
);
150+
}
151+
}
152+
}
153+
}
154+
}
155+
}
156+
157+
expect(offending).toEqual([]);
158+
});
159+
160+
it('should register a meaningful number of tools (smoke check)', async () => {
161+
const server = new McpServer({ name: 'schema-test', version: '0.0.0' });
162+
registerAllTools(server);
163+
164+
const registered = (server as any)._registeredTools as Record<string, unknown>;
165+
const toolCount = Object.keys(registered).length;
166+
167+
// Sanity: we expect at least 30 tools across all categories. If this
168+
// drops significantly it likely means a registration function is broken.
169+
expect(toolCount).toBeGreaterThanOrEqual(30);
170+
});
171+
172+
it('no tool property uses z.tuple serialization (bare array items)', async () => {
173+
const server = new McpServer({ name: 'schema-test', version: '0.0.0' });
174+
registerAllTools(server);
175+
176+
const registered = (server as any)._registeredTools as
177+
Record<string, { inputSchema?: unknown }>;
178+
179+
const { toJsonSchemaCompat } = await import(
180+
'@modelcontextprotocol/sdk/server/zod-json-schema-compat.js'
181+
);
182+
183+
const tupleProps: string[] = [];
184+
185+
for (const [toolName, def] of Object.entries(registered)) {
186+
if (!def.inputSchema) continue;
187+
188+
const jsonSchema = toJsonSchemaCompat(def.inputSchema as never) as {
189+
properties?: Record<string, unknown>;
190+
};
191+
192+
for (const [propName, propSchema] of Object.entries(jsonSchema.properties ?? {})) {
193+
// A z.tuple([...]) serializes with `items` as an array (positional).
194+
// Standard z.array() serializes `items` as a single schema object.
195+
if (
196+
typeof propSchema === 'object' &&
197+
propSchema !== null &&
198+
'items' in propSchema &&
199+
Array.isArray((propSchema as { items: unknown }).items)
200+
) {
201+
tupleProps.push(`${toolName}.${propName}`);
202+
}
203+
}
204+
}
205+
206+
expect(
207+
tupleProps,
208+
`Tools with tuple-serialized properties (use z.object instead): ${tupleProps.join(', ')}`,
209+
).toEqual([]);
210+
});
211+
});

0 commit comments

Comments
 (0)