fix(cli): repair tools cache thrashing and add negative caching

The bare mcp-cli-ent invocation re-discovered tools from every server on
each call because the cache gate required all enabled servers present. One
failing server made the cache unusable, costing 20-30s per invocation.

The discovery path now serves valid cache hits instantly and only contacts
servers that are missing, expired, or past their negative-cache window.
Failed servers are cached for five minutes and surfaced on stderr instead
of vanishing silently. The cache map is merged in place rather than
overwritten, so a transient failure on one server no longer evicts good
entries for the rest. Each discovery worker gets its own timeout that
prefers serverConfig.Timeout and falls back to the global flag.

Bumps VERSION to 1.3.0.

Author: EstebanForge

CHANGELOG.md

@@ -2,6 +2,20 @@
 
 All notable changes to MCP CLI-Ent will be documented in this file.
 
+<!-- RELEASE:START 1.3.0 -->
+## [1.3.0] - 2026-07-08
+
+### Fixed
+
+- **Tools cache no longer thrashes on every invocation.** Bare `mcp-cli-ent` was re-discovering tools from all servers on every call (20-30s per invocation) because the cache gate required every enabled server to be present. A single failing server made the whole cache unusable. The discovery path now serves valid cache hits instantly and only contacts servers that are missing, expired, or past their negative-cache window.
+- **Negative caching with visibility.** Servers that fail to connect are cached as failed for 5 minutes so they stop forcing full re-discovery. They are no longer dropped silently: a stderr line (`<server>: (skipped: recently failed; retry in <duration> or use --refresh)`) reports their status instead of letting them vanish from the output.
+- **Cache is now merged, not overwritten.** Previously `SaveToolsToCache` wrote only the current run's successes, evicting previously-good entries on any transient failure (the cache shrank over time). The cache map is now updated in place, so a sibling failure never drops a good entry for another server.
+- **`--clear-cache` / `--refresh` no longer delete the cache file.** They force live discovery for enabled servers, whose fresh results overwrite only their own keys. Cached entries for dormant or disabled servers are preserved.
+- **Per-worker discovery timeout now honors server config.** Each discovery worker gets a `context.WithTimeout` that prefers `serverConfig.Timeout` and falls back to the global `--timeout` flag, so one hanging server can no longer stall the whole invocation up to the 30s transport default.
+- **Cache write failures surface under `--verbose`.** A failed `SaveToolsToCache` previously failed silently, causing mysterious perpetual slowness; it now logs a warning.
+
+<!-- RELEASE:END 1.3.0 -->
+
 <!-- RELEASE:START 1.2.2 -->
 ## [1.2.2] - 2026-06-08
 

VERSION

@@ -1 +1 @@
-1.2.2
+1.3.0

internal/cli/root.go

@@ -29,10 +29,13 @@ var (
 	searchQuery  string
 )
 
-// ToolsCacheEntry represents a cached tool listing for a server
+// ToolsCacheEntry represents a cached tool listing for a server.
+// Failed entries act as a short-lived negative cache so a broken server
+// does not force a full re-discovery on every invocation.
 type ToolsCacheEntry struct {
 	Tools      []mcp.Tool `json:"tools"`
 	LastUpdate time.Time  `json:"lastUpdate"`
+	Failed     bool       `json:"failed,omitempty"`
 }
 
 // ToolsCache represents the full cache structure
@@ -41,8 +44,9 @@ type ToolsCache struct {
 }
 
 const (
-	CacheFileName = "tools_cache.json"
-	CacheTTL      = 30 * 24 * time.Hour // Cache expires after 30 days
+	CacheFileName    = "tools_cache.json"
+	CacheTTL         = 30 * 24 * time.Hour // Successful entries expire after 30 days
+	NegativeCacheTTL = 5 * time.Minute     // Failed entries: retry the server after 5 min
 )
 
 // rootCmd represents the base command when called without any subcommands
@@ -153,107 +157,116 @@ func showRootHelpWithServers(cmd *cobra.Command) error {
 		return nil
 	}
 
-	// If clearCache or refreshCache is set, clear cache file
-	// These flags are aliases - both trigger cache refresh
+	// --refresh / --clear-cache are aliases. They force live discovery for
+	// enabled servers, whose fresh results overwrite only their own cache
+	// keys. The file itself is never deleted, so dormant/disabled servers
+	// keep their cached entries (true merge semantics).
 	if clearCache || refreshCache {
-		cachePath, err := GetCachePath()
-		if err == nil {
-			_ = os.Remove(cachePath)
-			fmt.Println("Cache cleared.")
-		}
-		// Force refresh after clearing
 		refreshCache = true
 		clearCache = true
 	}
 
+	// Load existing cache. We MERGE into it rather than overwrite, so a
+	// transient failure on one server never evicts good entries for others.
 	cache, err := LoadToolsFromCache()
-	if err != nil {
-		cache = nil
+	if err != nil || cache == nil {
+		cache = &ToolsCache{Servers: make(map[string]ToolsCacheEntry)}
+	}
+	if cache.Servers == nil {
+		cache.Servers = make(map[string]ToolsCacheEntry)
 	}
-	useCache := cache != nil && !refreshCache && !clearCache
 
-	var totalTools int
-	var toolsByServer map[string][]mcp.Tool
+	toolsByServer := make(map[string][]mcp.Tool)
+	var toDiscover []string
+	now := time.Now()
 
-	if useCache {
-		// Check if all servers are in cache
-		toolsByServer = make(map[string][]mcp.Tool)
-		allCached := true
-		for serverName := range enabledServers {
-			if entry, ok := cache.Servers[serverName]; ok {
-				toolsByServer[serverName] = entry.Tools
-			} else {
-				allCached = false
-				break
+	// Partition enabled servers: serve valid cache hits immediately, queue
+	// the rest (missing, expired, or failed-past-TTL) for live discovery.
+	for serverName := range enabledServers {
+		entry, cached := cache.Servers[serverName]
+		if cached && !refreshCache && !clearCache {
+			ttl := CacheTTL
+			if entry.Failed {
+				ttl = NegativeCacheTTL
 			}
-		}
-
-		if !allCached {
-			useCache = false
-		} else {
-			// Count tools from cache
-			for _, tools := range toolsByServer {
-				totalTools += len(tools)
+			if now.Sub(entry.LastUpdate) <= ttl {
+				// Still valid. Negative entries yield no tools but are
+				// surfaced so a failing server does not vanish silently.
+				if entry.Failed {
+					fmt.Fprintf(os.Stderr, "%s: (skipped: recently failed; retry in %s or use --refresh)\n",
+						serverName, (ttl - now.Sub(entry.LastUpdate)).Round(time.Second))
+				} else {
+					toolsByServer[serverName] = entry.Tools
+				}
+				continue
 			}
 		}
+		toDiscover = append(toDiscover, serverName)
 	}
 
-	if !useCache {
-		// Discover tools from all servers
+	// Discover only what we could not serve from cache. Each worker gets its
+	// own deadline so one hanging server cannot stall the whole invocation.
+	if len(toDiscover) > 0 {
 		factory, err := getSessionAwareClientFactory()
 		if err != nil {
 			fmt.Fprintf(os.Stderr, "Error: failed to create client factory: %v\n", err)
 			fmt.Fprintln(os.Stderr, "Run 'mcp-cli-ent list-servers' to see available servers")
 			return nil
 		}
 
-		toolsByServer = make(map[string][]mcp.Tool)
 		var wg sync.WaitGroup
 		var mu sync.Mutex
-
-		// Start workers for parallel discovery
-		for serverName := range enabledServers {
+		for _, name := range toDiscover {
 			wg.Add(1)
 			go func(name string) {
 				defer wg.Done()
-				ctx := context.Background()
 				serverConfig := enabledServers[name]
 
+				// Honor a server-specific timeout if set, else the global flag.
+				serverTimeout := timeout
+				if serverConfig.Timeout > 0 {
+					serverTimeout = serverConfig.Timeout
+				}
+				ctx, cancel := context.WithTimeout(context.Background(), time.Duration(serverTimeout)*time.Second)
+				defer cancel()
+
 				mcpClient, err := factory.CreateClient(name, serverConfig)
 				if err != nil {
 					fmt.Fprintf(os.Stderr, "%s: (failed to connect: %v)\n", name, err)
+					mu.Lock()
+					cache.Servers[name] = ToolsCacheEntry{Failed: true, LastUpdate: time.Now()}
+					mu.Unlock()
 					return
 				}
 
 				tools, err := mcpClient.ListTools(ctx)
 				_ = mcpClient.Close()
 				if err != nil {
 					fmt.Fprintf(os.Stderr, "%s: (failed to list tools: %v)\n", name, err)
+					mu.Lock()
+					cache.Servers[name] = ToolsCacheEntry{Failed: true, LastUpdate: time.Now()}
+					mu.Unlock()
 					return
 				}
 
 				mu.Lock()
 				toolsByServer[name] = tools
+				cache.Servers[name] = ToolsCacheEntry{Tools: tools, LastUpdate: time.Now()}
 				mu.Unlock()
-			}(serverName)
+			}(name)
 		}
-
-		// Wait for all workers to complete
 		wg.Wait()
 
-		// Count total tools and build cache
-		totalTools = 0
-		newCache := &ToolsCache{Servers: make(map[string]ToolsCacheEntry)}
-		for serverName, tools := range toolsByServer {
-			totalTools += len(tools)
-			newCache.Servers[serverName] = ToolsCacheEntry{
-				Tools:      tools,
-				LastUpdate: time.Now(),
-			}
+		// Persist the merged cache: fresh successes + retained hits + new
+		// negative entries. A sibling failure never drops a good entry.
+		if err := SaveToolsToCache(cache); err != nil && verbose {
+			fmt.Fprintf(os.Stderr, "Warning: failed to save tools cache: %v\n", err)
 		}
+	}
 
-		// Save to cache
-		_ = SaveToolsToCache(newCache)
+	totalTools := 0
+	for _, tools := range toolsByServer {
+		totalTools += len(tools)
 	}
 
 	// Filter by search query if provided