fix(quarantine): non-blocking inspection with circuit breaker (issue #105)

Implements non-blocking quarantine inspection with circuit breaker pattern
to prevent UI freezes and cascading failures with unstable MCP servers.

## Problem (Issue #105)
Quarantine inspection used blocking 20s wait loop that froze the MCP
handler thread, causing dashboard hangs and requiring forced kills.
Unstable servers (like JetBrains MCP) triggered repeated timeouts.

## Solution

### 1. Non-blocking Connection Wait (mcp.go:1177-1269)
- Replaced blocking time.Sleep() loop with goroutine + channels
- Proper context cancellation support
- Progress logging every 2 seconds
- 20-second timeout with clean error messages

### 2. Circuit Breaker Pattern (supervisor.go:945-1059)
- Tracks consecutive inspection failures per server
- 3-failure threshold triggers 5-minute cooldown
- Prevents repeated timeout attempts on unstable servers
- Success resets failure counter
- Methods: CanInspect(), RecordInspectionFailure(), RecordInspectionSuccess()

### 3. Config Sync Fix (mcp.go:1568-1579)
- UpdateConfig() called after adding server
- Fixes ConfigService synchronization in test environments
- Ensures supervisor sees newly added quarantined servers

## Test Results

### Unit Tests (exemption_test.go)
- TestCircuitBreaker: 3-failure threshold ✅
- TestCircuitBreakerReset: Success resets counter ✅
- TestExemptionExpiry: Time-based expiration ✅

### Real-World Test (idea-ide-local)
- Calls 1-3: Each took ~20s (timeout), recorded failures ✅
- Call 4: Returned instantly with circuit breaker message ✅
- Cooldown: 5-minute protection active ✅

### E2E Test Updates
- Fixed tool name: upstream_servers → quarantine_security
- Improved error handling and debugging
- Better content type assertions

## Impact
- ✅ No more thread blocking during inspection
- ✅ Dashboard remains responsive during 20s timeout
- ✅ Circuit breaker prevents cascading failures
- ✅ Clear error messages reference issue #105

## Related
- Addresses feedback from PR #110 (SSE stability)
- Complements connection loss detection improvements
- Part of upstream stability improvements

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: Dumbris

internal/runtime/supervisor/exemption_test.go

@@ -0,0 +1,89 @@
+package supervisor
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"go.uber.org/zap"
+)
+
+// TestCircuitBreaker tests the circuit breaker pattern for inspection failures
+func TestCircuitBreaker(t *testing.T) {
+	logger, _ := zap.NewDevelopment()
+
+	// Test failure recording
+	s := &Supervisor{
+		logger:             logger,
+		inspectionFailures: make(map[string]*inspectionFailureInfo),
+	}
+
+	serverName := "test-server"
+
+	// Record 3 failures
+	for i := 0; i < 3; i++ {
+		s.RecordInspectionFailure(serverName)
+	}
+
+	// Check that circuit breaker is now active
+	allowed, reason, cooldown := s.CanInspect(serverName)
+	assert.False(t, allowed, "Should not allow inspection after 3 failures")
+	assert.Contains(t, reason, "Circuit breaker active")
+	assert.Greater(t, cooldown, time.Duration(0), "Should have cooldown remaining")
+
+	// Verify cooldown duration is approximately 5 minutes
+	assert.InDelta(t, inspectionCooldown.Seconds(), cooldown.Seconds(), 1.0, "Cooldown should be ~5 minutes")
+}
+
+// TestCircuitBreakerReset tests that successful inspection resets the failure counter
+func TestCircuitBreakerReset(t *testing.T) {
+	logger, _ := zap.NewDevelopment()
+
+	s := &Supervisor{
+		logger:             logger,
+		inspectionFailures: make(map[string]*inspectionFailureInfo),
+	}
+
+	serverName := "test-server"
+
+	// Record 2 failures
+	s.RecordInspectionFailure(serverName)
+	s.RecordInspectionFailure(serverName)
+
+	// Verify we're still allowed (under threshold)
+	allowed, _, _ := s.CanInspect(serverName)
+	assert.True(t, allowed, "Should still allow inspection with 2 failures")
+
+	// Record success - should reset counter
+	s.RecordInspectionSuccess(serverName)
+
+	// Verify failure counter was reset
+	failures, inCooldown, _ := s.GetInspectionStats(serverName)
+	assert.Equal(t, 0, failures, "Failure counter should be reset after success")
+	assert.False(t, inCooldown, "Should not be in cooldown after success")
+}
+
+// TestExemptionExpiry tests that exemptions expire correctly
+func TestExemptionExpiry(t *testing.T) {
+	logger, _ := zap.NewDevelopment()
+
+	s := &Supervisor{
+		logger:                logger,
+		inspectionExemptions:  make(map[string]time.Time),
+		inspectionFailures:    make(map[string]*inspectionFailureInfo),
+	}
+
+	serverName := "test-server"
+
+	// Grant exemption with short duration
+	s.inspectionExemptions[serverName] = time.Now().Add(100 * time.Millisecond)
+
+	// Should be exempted immediately
+	assert.True(t, s.IsInspectionExempted(serverName), "Should be exempted immediately")
+
+	// Wait for expiry
+	time.Sleep(150 * time.Millisecond)
+
+	// Should no longer be exempted
+	assert.False(t, s.IsInspectionExempted(serverName), "Should not be exempted after expiry")
+}

internal/runtime/supervisor/supervisor.go

@@ -47,12 +47,23 @@ type Supervisor struct {
 	inspectionExemptions   map[string]time.Time
 	inspectionExemptionsMu sync.RWMutex
 
+	// Circuit breaker for inspection failures (Phase 2: Issue #105 stability)
+	inspectionFailures   map[string]*inspectionFailureInfo
+	inspectionFailuresMu sync.RWMutex
+
 	// Lifecycle
 	ctx    context.Context
 	cancel context.CancelFunc
 	wg     sync.WaitGroup
 }
 
+// inspectionFailureInfo tracks inspection failures for circuit breaker pattern
+type inspectionFailureInfo struct {
+	consecutiveFailures int
+	lastFailureTime     time.Time
+	cooldownUntil       time.Time
+}
+
 // UpstreamInterface defines the interface for upstream adapters.
 type UpstreamInterface interface {
 	AddServer(name string, cfg *config.ServerConfig) error
@@ -84,6 +95,7 @@ func New(configSvc *configsvc.Service, upstream UpstreamInterface, logger *zap.L
 		eventCh:              make(chan Event, 500), // Phase 6: Increased buffer for async operations
 		listeners:            make([]chan Event, 0),
 		inspectionExemptions: make(map[string]time.Time),
+		inspectionFailures:   make(map[string]*inspectionFailureInfo),
 		ctx:                  ctx,
 		cancel:               cancel,
 	}
@@ -929,3 +941,119 @@ func (s *Supervisor) IsInspectionExempted(serverName string) bool {
 
 	return true
 }
+
+// ===== Circuit Breaker for Inspection Failures (Issue #105) =====
+
+const (
+	maxInspectionFailures  = 3                // Max consecutive failures before cooldown
+	inspectionCooldown     = 5 * time.Minute  // Cooldown duration after max failures
+	failureResetTimeout    = 10 * time.Minute // Reset counter if no failures for this long
+)
+
+// CanInspect checks if inspection is allowed for a server (circuit breaker)
+// Returns (allowed bool, reason string, cooldownRemaining time.Duration)
+func (s *Supervisor) CanInspect(serverName string) (bool, string, time.Duration) {
+	s.inspectionFailuresMu.RLock()
+	defer s.inspectionFailuresMu.RUnlock()
+
+	info, exists := s.inspectionFailures[serverName]
+	if !exists {
+		// No failure history - allow inspection
+		return true, "", 0
+	}
+
+	now := time.Now()
+
+	// Check if cooldown is active
+	if now.Before(info.cooldownUntil) {
+		remaining := info.cooldownUntil.Sub(now)
+		reason := fmt.Sprintf("Server '%s' has failed inspection %d times. Circuit breaker active - please wait %v before retrying. This prevents cascading failures with unstable servers (see issue #105).",
+			serverName, info.consecutiveFailures, remaining.Round(time.Second))
+		return false, reason, remaining
+	}
+
+	// Check if failures should be reset (no failures for failureResetTimeout)
+	if now.Sub(info.lastFailureTime) > failureResetTimeout {
+		// Failures are old - will be reset on next inspection
+		return true, "", 0
+	}
+
+	// Within failure window but not in cooldown
+	return true, "", 0
+}
+
+// RecordInspectionFailure records an inspection failure for circuit breaker
+func (s *Supervisor) RecordInspectionFailure(serverName string) {
+	s.inspectionFailuresMu.Lock()
+	defer s.inspectionFailuresMu.Unlock()
+
+	now := time.Now()
+
+	info, exists := s.inspectionFailures[serverName]
+	if !exists {
+		info = &inspectionFailureInfo{}
+		s.inspectionFailures[serverName] = info
+	}
+
+	// Reset counter if last failure was too long ago
+	if now.Sub(info.lastFailureTime) > failureResetTimeout {
+		info.consecutiveFailures = 0
+	}
+
+	info.consecutiveFailures++
+	info.lastFailureTime = now
+
+	s.logger.Warn("Inspection failure recorded",
+		zap.String("server", serverName),
+		zap.Int("consecutive_failures", info.consecutiveFailures),
+		zap.Int("max_before_cooldown", maxInspectionFailures))
+
+	// Activate cooldown if max failures reached
+	if info.consecutiveFailures >= maxInspectionFailures {
+		info.cooldownUntil = now.Add(inspectionCooldown)
+		s.logger.Error("⚠️ Inspection circuit breaker activated - too many failures",
+			zap.String("server", serverName),
+			zap.Int("failures", info.consecutiveFailures),
+			zap.Duration("cooldown", inspectionCooldown),
+			zap.Time("cooldown_until", info.cooldownUntil),
+			zap.String("issue", "#105 - preventing cascading failures"))
+	}
+}
+
+// RecordInspectionSuccess records a successful inspection, resetting failure counter
+func (s *Supervisor) RecordInspectionSuccess(serverName string) {
+	s.inspectionFailuresMu.Lock()
+	defer s.inspectionFailuresMu.Unlock()
+
+	info, exists := s.inspectionFailures[serverName]
+	if !exists {
+		return
+	}
+
+	if info.consecutiveFailures > 0 {
+		s.logger.Info("Inspection succeeded - resetting failure counter",
+			zap.String("server", serverName),
+			zap.Int("previous_failures", info.consecutiveFailures))
+	}
+
+	// Reset failure counter
+	delete(s.inspectionFailures, serverName)
+}
+
+// GetInspectionStats returns inspection failure statistics for a server
+func (s *Supervisor) GetInspectionStats(serverName string) (failures int, inCooldown bool, cooldownRemaining time.Duration) {
+	s.inspectionFailuresMu.RLock()
+	defer s.inspectionFailuresMu.RUnlock()
+
+	info, exists := s.inspectionFailures[serverName]
+	if !exists {
+		return 0, false, 0
+	}
+
+	now := time.Now()
+	if now.Before(info.cooldownUntil) {
+		return info.consecutiveFailures, true, info.cooldownUntil.Sub(now)
+	}
+
+	return info.consecutiveFailures, false, 0
+}

internal/server/e2e_test.go

@@ -1029,29 +1029,43 @@ func TestE2E_InspectQuarantined(t *testing.T) {
 
 	t.Log("🔍 Calling inspect_quarantined for quarantined-server...")
 
-	// Call inspect_quarantined
+	// Call inspect_quarantined (use quarantine_security tool, not upstream_servers)
 	inspectRequest := mcp.CallToolRequest{}
-	inspectRequest.Params.Name = "upstream_servers"
+	inspectRequest.Params.Name = "quarantine_security"
 	inspectRequest.Params.Arguments = map[string]interface{}{
 		"operation": "inspect_quarantined",
 		"name":      "quarantined-server",
 	}
 
 	inspectResult, err := mcpClient.CallTool(ctx, inspectRequest)
 	require.NoError(t, err, "inspect_quarantined should not return error")
+
+	// Debug: Print all content items with their types
+	t.Logf("📋 Inspection result - IsError: %v, Content count: %d", inspectResult.IsError, len(inspectResult.Content))
+	for i, content := range inspectResult.Content {
+		t.Logf("Content[%d] type: %T", i, content)
+		// Handle both pointer and value types
+		if textContent, ok := content.(*mcp.TextContent); ok {
+			t.Logf("Content[%d] text (pointer): %s", i, textContent.Text)
+		} else if textContent, ok := content.(mcp.TextContent); ok {
+			t.Logf("Content[%d] text (value): %s", i, textContent.Text)
+		}
+	}
+
 	if inspectResult.IsError {
-		// Print the error for debugging
+		// Print the error for debugging - handle both pointer and value types
 		for _, content := range inspectResult.Content {
 			if textContent, ok := content.(*mcp.TextContent); ok {
-				t.Logf("❌ Error from inspect_quarantined: %s", textContent.Text)
+				t.Logf("❌ Error from inspect_quarantined (pointer): %s", textContent.Text)
+			} else if textContent, ok := content.(mcp.TextContent); ok {
+				t.Logf("❌ Error from inspect_quarantined (value): %s", textContent.Text)
 			}
 		}
+		t.Fatal("inspect_quarantined returned an error - see logs above")
 	}
-	assert.False(t, inspectResult.IsError, "inspect_quarantined should succeed")
 
 	// Verify result contains tool data
 	require.NotEmpty(t, inspectResult.Content, "Result should have content")
-	t.Logf("✅ Inspection result received: %d content items", len(inspectResult.Content))
 
 	// Verify the result contains information about the tools
 	var resultText string