Addressed Claude PR review comments: Added tsx dev dependency, beefed up process termination (possible leak on Windows), beefed up http server cleanup (close all connections), removed unused hasValidJsonOutput, reduced CLI timeout to give it breathing room with vitest timeout.

Author: BobDickinson

cli/__tests__/helpers/assertions.ts

@@ -50,17 +50,3 @@ export function expectJsonStructure(result: CliResult, expectedKeys: string[]) {
   });
   return json;
 }
-
-/**
- * Check if output contains valid JSON (for tools/resources/prompts responses)
- */
-export function hasValidJsonOutput(output: string): boolean {
-  return (
-    output.includes('"tools"') ||
-    output.includes('"resources"') ||
-    output.includes('"prompts"') ||
-    output.includes('"content"') ||
-    output.includes('"messages"') ||
-    output.includes('"contents"')
-  );
-}

cli/__tests__/helpers/cli-runner.ts

@@ -41,22 +41,26 @@ export async function runCli(
     let stderr = "";
     let resolved = false;
 
-    // Default timeout of 12 seconds (less than vitest's 15s)
-    const timeoutMs = options.timeout ?? 12000;
+    // Default timeout of 10 seconds (less than vitest's 15s)
+    const timeoutMs = options.timeout ?? 10000;
     const timeout = setTimeout(() => {
       if (!resolved) {
         resolved = true;
         // Kill the process and all its children
         try {
           if (process.platform === "win32") {
-            child.kill();
+            child.kill("SIGTERM");
           } else {
             // On Unix, kill the process group
             process.kill(-child.pid!, "SIGTERM");
           }
         } catch (e) {
-          // Process might already be dead
-          child.kill();
+          // Process might already be dead, try direct kill
+          try {
+            child.kill("SIGKILL");
+          } catch (e2) {
+            // Process is definitely dead
+          }
         }
         reject(new Error(`CLI command timed out after ${timeoutMs}ms`));
       }

cli/__tests__/helpers/instrumented-server.ts

@@ -422,6 +422,8 @@ export class InstrumentedServer {
 
     if (this.httpServer) {
       return new Promise((resolve) => {
+        // Force close all connections
+        this.httpServer!.closeAllConnections?.();
         this.httpServer!.close(() => {
           this.httpServer = undefined;
           resolve();

cli/package.json

@@ -26,6 +26,7 @@
   },
   "devDependencies": {
     "@types/express": "^5.0.6",
+    "tsx": "^4.7.0",
     "vitest": "^4.0.17"
   },
   "dependencies": {