mcp improvements

Author: Eric Oliver

docs/product-stories/cli-hanging-comprehensive-fix.md

@@ -0,0 +1,374 @@
+# CLI Hanging Fix - Comprehensive Solution
+
+## Overview
+
+Fix the CLI hanging issue by addressing task completion detection, MCP disposal, and process exit enforcement.
+
+## Problem Statement
+
+The CLI hangs after processing batch commands and never exits naturally, requiring users to manually terminate with Ctrl+C. This affects user experience and prevents automated usage.
+
+## Root Cause Analysis
+
+### 1. Task Completion Detection Issues
+
+- BatchProcessor relies on event-driven completion detection
+- Simple Q&A tasks may not emit expected completion events
+- Complex timeout logic has edge cases
+- No fallback for non-tool response scenarios
+
+### 2. MCP Server Disposal Problems
+
+- Stdio child processes (dotnet, npx) don't terminate cleanly
+- 3-second disposal timeout insufficient for some processes
+- No force-kill mechanism for stubborn processes
+- Incomplete cleanup of event listeners and resources
+
+### 3. Event Loop Prevention
+
+- Timers, intervals, and event listeners keep process alive
+- No guaranteed exit mechanism
+- Async operations may hang indefinitely
+- Missing force-exit as last resort
+
+## Solution Architecture
+
+### Phase 1: Task Completion Detection Enhancement
+
+**File: `src/cli/commands/batch.ts`**
+
+#### Changes:
+
+1. **Response-based Completion Detection**
+
+    - Detect completion when meaningful response is generated
+    - Don't rely solely on task events for simple Q&A
+    - Add completion heuristics for different response types
+
+2. **Improved Timeout Logic**
+
+    - Simplify sliding timeout implementation
+    - Add response completion triggers
+    - Better handling of non-interactive responses
+
+3. **Fallback Completion**
+    - Detect when response generation completes
+    - Set maximum response time limits
+    - Auto-complete for information queries
+
+#### Implementation:
+
+```typescript
+// Enhanced completion detection
+private detectResponseCompletion(response: string): boolean {
+    // Detect complete responses for information queries
+    const informationIndicators = [
+        'Available MCP servers',
+        'Connected MCP servers',
+        'No MCP servers',
+        'I have access to'
+    ];
+
+    return informationIndicators.some(indicator =>
+        response.toLowerCase().includes(indicator.toLowerCase())
+    );
+}
+
+// Response-triggered completion
+private setupResponseCompletionDetection(task: Task): void {
+    let responseBuffer = '';
+
+    task.on('message', (event) => {
+        if (event.action === 'response') {
+            responseBuffer += event.message?.text || '';
+
+            // Check if response appears complete
+            if (this.detectResponseCompletion(responseBuffer)) {
+                // Trigger completion after brief delay
+                setTimeout(() => {
+                    this.completeTask('Response completion detected');
+                }, 1000);
+            }
+        }
+    });
+}
+```
+
+### Phase 2: MCP Disposal Enhancement
+
+**Files: `src/cli/services/CLIMcpService.ts`, `src/cli/connections/BaseMcpConnection.ts`**
+
+#### Changes:
+
+1. **Force Process Termination**
+
+    - Add SIGTERM followed by SIGKILL for stdio processes
+    - Increase disposal timeout to 5 seconds
+    - Track child process PIDs for forced cleanup
+
+2. **Enhanced Connection Cleanup**
+
+    - Properly close all transport connections
+    - Clear all event listeners and timers
+    - Force-dispose stubborn connections
+
+3. **Disposal Monitoring**
+    - Log disposal progress for debugging
+    - Track which processes are hanging
+    - Provide disposal status feedback
+
+#### Implementation:
+
+```typescript
+// Enhanced stdio process termination
+async forceTerminateStdioProcess(): Promise<void> {
+    const transport = this.transport as StdioClientTransport;
+    const childProcess = transport.process;
+
+    if (childProcess && !childProcess.killed) {
+        // Graceful termination first
+        childProcess.kill('SIGTERM');
+
+        // Wait up to 2 seconds for graceful exit
+        await new Promise((resolve) => {
+            const timeout = setTimeout(() => {
+                // Force kill if still running
+                if (!childProcess.killed) {
+                    childProcess.kill('SIGKILL');
+                }
+                resolve(void 0);
+            }, 2000);
+
+            childProcess.on('exit', () => {
+                clearTimeout(timeout);
+                resolve(void 0);
+            });
+        });
+    }
+}
+
+// Enhanced disposal with force cleanup
+async dispose(): Promise<void> {
+    const disposalTimeout = 5000; // 5 seconds
+
+    const disposalPromise = Promise.allSettled([
+        ...Array.from(this.connections.keys()).map(id =>
+            this.forceDisconnectServer(id)
+        )
+    ]);
+
+    // Race disposal against timeout
+    const timeoutPromise = new Promise((_, reject) =>
+        setTimeout(() => reject(new Error('Disposal timeout')), disposalTimeout)
+    );
+
+    try {
+        await Promise.race([disposalPromise, timeoutPromise]);
+    } catch (error) {
+        console.warn('Disposal timeout, forcing cleanup...');
+        // Force cleanup will happen in finally block
+    } finally {
+        // Clear all remaining references
+        this.connections.clear();
+        this.healthCheckers.clear();
+        this.isDisposed = true;
+    }
+}
+```
+
+### Phase 3: Process Exit Enforcement
+
+**File: `src/cli/index.ts`**
+
+#### Changes:
+
+1. **Guaranteed Exit Mechanism**
+
+    - Force exit after maximum operation time
+    - Clean shutdown with fallback to process.exit()
+    - Exit code management for different scenarios
+
+2. **Event Loop Cleanup**
+
+    - Clear all timers and intervals
+    - Remove event listeners
+    - Close remaining handles
+
+3. **Exit Coordination**
+    - Coordinate between task completion and MCP disposal
+    - Ensure proper cleanup order
+    - Log exit process for debugging
+
+#### Implementation:
+
+```typescript
+// Global exit enforcement
+private setupGlobalExitEnforcement(): void {
+    const maxExecutionTime = 60000; // 1 minute max for any operation
+
+    const forceExitTimer = setTimeout(() => {
+        console.warn('Force exiting due to execution timeout');
+        process.exit(1);
+    }, maxExecutionTime);
+
+    // Clear timer on normal completion
+    process.on('beforeExit', () => {
+        clearTimeout(forceExitTimer);
+    });
+}
+
+// Enhanced cleanup sequence
+private async performComprehensiveCleanup(): Promise<void> {
+    const cleanupTasks = [
+        this.disposeMcpServices(),
+        this.clearGlobalTimers(),
+        this.removeEventListeners(),
+        this.closeOpenHandles()
+    ];
+
+    // Execute cleanup with timeout
+    try {
+        await Promise.race([
+            Promise.allSettled(cleanupTasks),
+            new Promise((_, reject) =>
+                setTimeout(() => reject(new Error('Cleanup timeout')), 5000)
+            )
+        ]);
+    } catch (error) {
+        console.warn('Cleanup timeout, forcing exit');
+    } finally {
+        // Guarantee exit
+        setTimeout(() => process.exit(0), 1000);
+    }
+}
+```
+
+### Phase 4: Integration and Testing
+
+**Files: Multiple**
+
+#### Changes:
+
+1. **Coordinated Shutdown**
+
+    - Integrate all cleanup phases
+    - Proper error handling and logging
+    - Graceful degradation for cleanup failures
+
+2. **Debug Logging**
+
+    - Add comprehensive debug logging for exit process
+    - Track hanging operations
+    - Identify remaining event loop items
+
+3. **Testing Framework**
+    - Add tests for hanging scenarios
+    - Verify clean exit in various conditions
+    - Performance testing for disposal operations
+
+## Implementation Steps
+
+### Step 1: Task Completion Enhancement
+
+- [ ] Implement response-based completion detection
+- [ ] Add completion heuristics for information queries
+- [ ] Simplify timeout logic with fallback completion
+- [ ] Test with various question types
+
+### Step 2: MCP Disposal Enhancement
+
+- [ ] Add force termination for stdio processes
+- [ ] Implement comprehensive connection cleanup
+- [ ] Increase disposal timeouts and add monitoring
+- [ ] Test with multiple MCP server types
+
+### Step 3: Process Exit Enforcement
+
+- [ ] Implement guaranteed exit mechanisms
+- [ ] Add event loop cleanup utilities
+- [ ] Coordinate shutdown sequence
+- [ ] Add comprehensive debug logging
+
+### Step 4: Integration Testing
+
+- [ ] Test complete fix with original hanging scenario
+- [ ] Verify clean exit in various conditions
+- [ ] Performance test disposal operations
+- [ ] Add automated tests for regression prevention
+
+## Acceptance Criteria
+
+### Functional Requirements
+
+- [ ] CLI exits cleanly after batch command completion
+- [ ] No hanging processes or child processes remain
+- [ ] All MCP servers disconnect properly
+- [ ] Exit codes reflect operation success/failure
+
+### Performance Requirements
+
+- [ ] Total exit time < 10 seconds under normal conditions
+- [ ] Force exit triggers within 1 minute maximum
+- [ ] MCP disposal completes within 5 seconds
+- [ ] No memory leaks or resource leaks
+
+### Reliability Requirements
+
+- [ ] 100% exit success rate for batch commands
+- [ ] Graceful handling of disposal failures
+- [ ] Proper cleanup even when services hang
+- [ ] Comprehensive error logging and debugging info
+
+## Testing Strategy
+
+### Unit Tests
+
+- Task completion detection logic
+- MCP disposal mechanisms
+- Process cleanup utilities
+- Timeout and fallback logic
+
+### Integration Tests
+
+- End-to-end batch command execution
+- Multiple MCP server scenarios
+- Error condition handling
+- Resource cleanup verification
+
+### Regression Tests
+
+- Original hanging scenario
+- Various MCP server configurations
+- Different task types and responses
+- Timeout and error conditions
+
+## Risk Mitigation
+
+### Potential Issues
+
+- Force-killing processes may cause data loss
+- Aggressive timeouts might interrupt legitimate operations
+- Changes might affect interactive mode behavior
+
+### Mitigation Strategies
+
+- Graduated escalation (graceful -> force -> kill)
+- Separate timeout values for different operations
+- Extensive testing in both batch and interactive modes
+- Rollback plan for compatibility issues
+
+## Success Metrics
+
+### Primary Metrics
+
+- CLI hanging incidents: 0%
+- Clean exit success rate: 100%
+- User-reported hanging issues: 0
+
+### Secondary Metrics
+
+- Average exit time: < 3 seconds
+- MCP disposal success rate: > 99%
+- Resource cleanup success: 100%
+
+This comprehensive fix addresses all identified root causes while maintaining system reliability and user experience.