fix: address critical Cursor review comments

- Fix missing null check in discover_tools.ts for server.server access
- Fix race condition in command.ts for detached processes by using resolved flag
- Remove duplicate validation in UI testing tools (key_sequence, long_press) since createTypedTool already validates via Zod schemas
- Clean up unused imports

Author: cameroncooke

src/mcp/tools/discovery/discover_tools.ts

@@ -58,7 +58,8 @@ export async function discover_toolsLogic(
     }
 
     // 1. Check for sampling capability
-    if (!server.server.getClientCapabilities()?.sampling) {
+    const clientCapabilities = server.server?.getClientCapabilities?.();
+    if (!clientCapabilities?.sampling) {
       log('warn', 'Client does not support sampling capability');
       return createTextResponse(
         'Your client does not support the sampling feature required for dynamic tool discovery. ' +
@@ -99,6 +100,9 @@ Each workflow contains ALL tools needed for its complete development workflow -
 
     // 4. Send sampling request
     log('debug', 'Sending sampling request to client LLM');
+    if (!server.server?.createMessage) {
+      throw new Error('Server does not support message creation');
+    }
     const samplingResult = await server.server.createMessage({
       messages: [{ role: 'user', content: { type: 'text', text: userPrompt } }],
       maxTokens: 200,

src/mcp/tools/ui-testing/key_sequence.ts

@@ -7,7 +7,7 @@
 import { z } from 'zod';
 import { ToolResponse } from '../../../types/common.js';
 import { log } from '../../../utils/index.js';
-import { validateRequiredParam, createTextResponse } from '../../../utils/index.js';
+import { createTextResponse } from '../../../utils/index.js';
 import {
   DependencyError,
   AxeError,
@@ -50,11 +50,6 @@ export async function key_sequenceLogic(
   },
 ): Promise<ToolResponse> {
   const toolName = 'key_sequence';
-  const simUuidValidation = validateRequiredParam('simulatorUuid', params.simulatorUuid);
-  if (!simUuidValidation.isValid) return simUuidValidation.errorResponse!;
-  const keyCodesValidation = validateRequiredParam('keyCodes', params.keyCodes);
-  if (!keyCodesValidation.isValid) return keyCodesValidation.errorResponse!;
-
   const { simulatorUuid, keyCodes, delay } = params;
   const commandArgs = ['key-sequence', '--keycodes', keyCodes.join(',')];
   if (delay !== undefined) {

src/mcp/tools/ui-testing/long_press.ts

@@ -8,7 +8,7 @@
 import { z } from 'zod';
 import { ToolResponse } from '../../../types/common.js';
 import { log } from '../../../utils/index.js';
-import { validateRequiredParam, createTextResponse } from '../../../utils/index.js';
+import { createTextResponse } from '../../../utils/index.js';
 import {
   DependencyError,
   AxeError,
@@ -52,15 +52,6 @@ export async function long_pressLogic(
   },
 ): Promise<ToolResponse> {
   const toolName = 'long_press';
-  const simUuidValidation = validateRequiredParam('simulatorUuid', params.simulatorUuid);
-  if (!simUuidValidation.isValid) return simUuidValidation.errorResponse!;
-  const xValidation = validateRequiredParam('x', params.x);
-  if (!xValidation.isValid) return xValidation.errorResponse!;
-  const yValidation = validateRequiredParam('y', params.y);
-  if (!yValidation.isValid) return yValidation.errorResponse!;
-  const durationValidation = validateRequiredParam('duration', params.duration);
-  if (!durationValidation.isValid) return durationValidation.errorResponse!;
-
   const { simulatorUuid, x, y, duration } = params;
   // AXe uses touch command with --down, --up, and --delay for long press
   const delayInSeconds = Number(duration) / 1000; // Convert ms to seconds

src/utils/command.ts

@@ -117,41 +117,55 @@ async function defaultExecutor(
       stderr += data.toString();
     });
 
-    childProcess.on('close', (code) => {
-      const success = code === 0;
-      const response: CommandResponse = {
-        success,
-        output: stdout,
-        error: success ? undefined : stderr,
-        process: childProcess,
-      };
-
-      resolve(response);
-    });
+    // For detached processes, handle differently to avoid race conditions
+    if (detached) {
+      // For detached processes, only wait for spawn success/failure
+      let resolved = false;
 
-    childProcess.on('error', (err) => {
-      reject(err);
-    });
+      childProcess.on('error', (err) => {
+        if (!resolved) {
+          resolved = true;
+          reject(err);
+        }
+      });
 
-    // For detached processes, resolve immediately after successful spawn
-    if (detached) {
       // Give a small delay to ensure the process starts successfully
       setTimeout(() => {
-        if (childProcess.pid) {
-          resolve({
-            success: true,
-            output: '', // No output yet for detached processes
-            process: childProcess,
-          });
-        } else {
-          resolve({
-            success: false,
-            output: '',
-            error: 'Failed to start detached process',
-            process: childProcess,
-          });
+        if (!resolved) {
+          resolved = true;
+          if (childProcess.pid) {
+            resolve({
+              success: true,
+              output: '', // No output for detached processes
+              process: childProcess,
+            });
+          } else {
+            resolve({
+              success: false,
+              output: '',
+              error: 'Failed to start detached process',
+              process: childProcess,
+            });
+          }
         }
       }, 100);
+    } else {
+      // For non-detached processes, handle normally
+      childProcess.on('close', (code) => {
+        const success = code === 0;
+        const response: CommandResponse = {
+          success,
+          output: stdout,
+          error: success ? undefined : stderr,
+          process: childProcess,
+        };
+
+        resolve(response);
+      });
+
+      childProcess.on('error', (err) => {
+        reject(err);
+      });
     }
   });
 }