More fixes for PR review feedback

Author: data-douser

server/dist/codeql-development-mcp-server.js

@@ -188612,12 +188612,13 @@ function buildEnhancedToolSchema(shape) {
         if (suggestion && !(suggestion in data) && !(suggestion in normalized)) {
           normalized[suggestion] = value;
         } else {
-          unknownEntries.push({ key, hint: suggestion });
+          const isDuplicate = !!suggestion && (suggestion in data || suggestion in normalized);
+          unknownEntries.push({ key, hint: suggestion, isDuplicate });
         }
       }
     }
-    for (const { key, hint } of unknownEntries) {
-      const message = hint ? `unknown property '${key}' \u2014 did you mean '${hint}'?` : `unknown property '${key}'`;
+    for (const { key, hint, isDuplicate } of unknownEntries) {
+      const message = isDuplicate && hint ? `duplicate property: both '${key}' and its canonical form '${hint}' were provided; use only '${hint}'` : hint ? `unknown property '${key}' \u2014 did you mean '${hint}'?` : `unknown property '${key}'`;
       ctx.addIssue({
         code: external_exports.ZodIssueCode.custom,
         message,

server/dist/codeql-development-mcp-server.js.map

@@ -78,7 +78,7 @@ export function buildEnhancedToolSchema(
     .passthrough()
     .transform((data, ctx) => {
       const normalized: Record<string, unknown> = {};
-      const unknownEntries: Array<{ key: string; hint?: string }> = [];
+      const unknownEntries: Array<{ key: string; hint?: string; isDuplicate: boolean }> = [];
 
       for (const [key, value] of Object.entries(data)) {
         if (knownKeys.has(key)) {
@@ -93,17 +93,20 @@ export function buildEnhancedToolSchema(
           } else {
             // Either no suggestion (truly unknown) or the canonical key is
             // already present.  Capture the suggestion so the error message
-            // can include a "did you mean?" hint.
-            unknownEntries.push({ key, hint: suggestion });
+            // can include a helpful hint.
+            const isDuplicate = !!suggestion && (suggestion in data || suggestion in normalized);
+            unknownEntries.push({ key, hint: suggestion, isDuplicate });
           }
         }
       }
 
-      // Report unknown / duplicate properties with "did you mean?" hints
-      for (const { key, hint } of unknownEntries) {
-        const message = hint
-          ? `unknown property '${key}' — did you mean '${hint}'?`
-          : `unknown property '${key}'`;
+      // Report unknown / duplicate properties with actionable messages
+      for (const { key, hint, isDuplicate } of unknownEntries) {
+        const message = isDuplicate && hint
+          ? `duplicate property: both '${key}' and its canonical form '${hint}' were provided; use only '${hint}'`
+          : hint
+            ? `unknown property '${key}' — did you mean '${hint}'?`
+            : `unknown property '${key}'`;
         ctx.addIssue({
           code: z.ZodIssueCode.custom,
           message,

server/src/lib/param-normalization.ts

@@ -204,15 +204,22 @@ describe('registerCLITool', () => {
 
     registerCLITool(mockServer, definition);
 
-    // registerTool is called with (name, config, callback)
+    // Verify the tool is registered with the right name, description, and a
+    // schema that normalizes camelCase keys (behaviour-level assertion instead
+    // of inspecting Zod internals like _def.typeName).
     expect(mockServer.registerTool).toHaveBeenCalledWith(
       'test_codeql_tool',
       expect.objectContaining({
         description: 'Test CodeQL tool',
-        inputSchema: expect.objectContaining({ _def: expect.objectContaining({ typeName: 'ZodEffects' }) }),
+        inputSchema: expect.any(Object),
       }),
       expect.any(Function)
     );
+
+    // Verify the schema normalizes a camelCase key to its canonical form
+    const registeredSchema = (mockServer.registerTool as ReturnType<typeof vi.fn>).mock.calls[0][1].inputSchema;
+    const parsed = registeredSchema.safeParse({ query: 'q', database: 'db' });
+    expect(parsed.success).toBe(true);
   });
 
   it('should register a QLT tool correctly', () => {
@@ -229,15 +236,19 @@ describe('registerCLITool', () => {
 
     registerCLITool(mockServer, definition);
 
-    // registerTool is called with (name, config, callback)
+    // Behaviour-level assertion: verify registration and schema parsing
     expect(mockServer.registerTool).toHaveBeenCalledWith(
       'test_qlt_tool',
       expect.objectContaining({
         description: 'Test QLT tool',
-        inputSchema: expect.objectContaining({ _def: expect.objectContaining({ typeName: 'ZodEffects' }) }),
+        inputSchema: expect.any(Object),
       }),
       expect.any(Function)
     );
+
+    const registeredSchema = (mockServer.registerTool as ReturnType<typeof vi.fn>).mock.calls[0][1].inputSchema;
+    const parsed = registeredSchema.safeParse({ language: 'java' });
+    expect(parsed.success).toBe(true);
   });
 
   it('should use custom result processor if provided', () => {

server/test/src/lib/cli-tool-registry.test.ts

@@ -179,7 +179,7 @@ describe('buildEnhancedToolSchema', () => {
       const messages = result.error.issues.map((i) => i.message);
       expect(messages).toEqual(
         expect.arrayContaining([
-          expect.stringContaining("unknown property 'sourceRoot' — did you mean 'source-root'?"),
+          expect.stringContaining("duplicate property: both 'sourceRoot' and its canonical form 'source-root' were provided; use only 'source-root'"),
         ]),
       );
     }
@@ -216,10 +216,9 @@ describe('buildEnhancedToolSchema', () => {
     }
   });
 
-  it('should include a "did you mean?" hint for unrecognized properties similar to known ones', () => {
-    // Provide a key that is NOT a direct camelCase/snake_case/kebab-case
-    // conversion of any known key — but IS a conversion in the OTHER direction.
-    // Example: the schema has camelCase "queryName" and the user sends "query-name"
+  it('should normalize kebab-case to camelCase when schema uses camelCase keys', () => {
+    // The schema has camelCase "queryName" and the user sends "query-name";
+    // normalization should silently convert to the canonical camelCase key.
     const camelSchema = buildEnhancedToolSchema({
       queryName: z.string().optional(),
     });