fix(mcp): address Bugbot findings on PR #19

Medium: insights.html onToolResult race
  The initialized flag was set to true BEFORE the try-catch in
  onToolResult. If parseToolResult threw or returned null, the flag was
  already set and the setTimeout fallback (which uses the more robust
  load() path) never fired — the UI locked on the loading spinner with
  no recovery. Now the flag only flips after a successful render.

Low: consolidate addToolWithMeta to delegate
  addToolWithMeta and addToolWithMetaTyped had ~25 lines of identical
  rate-limiting + audit-logging + tool-filtering wrapper. Any future fix
  would need to land in both places. Collapsed to a 1-line shim that
  calls the typed variant with struct{}, so the wrapper exists once.
  Call sites unchanged (s.addToolWithMeta still works).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: juergen-kc

internal/mcp/apps.go

@@ -124,47 +124,11 @@ func addToolWithMetaTyped[In any](s *Server, name, description string, meta mcp.
 	s.toolNames = append(s.toolNames, name)
 }
 
-// addToolWithMeta wraps mcp.AddTool with rate limiting, audit logging, and
-// tool filtering — same as addTool but also sets Meta on the tool definition.
-// This is used for MCP App tools that need _meta.ui.resourceUri.
+// addToolWithMeta is the no-args convenience wrapper for addToolWithMetaTyped.
+// Kept as a method on *Server for call-site ergonomics (s.addToolWithMeta(...))
+// since no-args App tools are the common case.
 func (s *Server) addToolWithMeta(name, description string, meta mcp.Meta, handler func(ctx context.Context, req *mcp.CallToolRequest, args struct{}) (*mcp.CallToolResult, any, error)) {
-	if !s.toolFilter.isAllowed(name) {
-		return
-	}
-
-	tool := &mcp.Tool{
-		Name:        name,
-		Description: description,
-		Meta:        meta,
-	}
-
-	wrappedHandler := func(ctx context.Context, req *mcp.CallToolRequest, args struct{}) (*mcp.CallToolResult, any, error) {
-		if err := s.limiter.allow(); err != nil {
-			s.auditLog.log(name, req.Params.Arguments, false, err.Error())
-			return errorResult(err.Error()), nil, nil
-		}
-
-		result, out, err := handler(ctx, req, args)
-
-		if err != nil {
-			s.auditLog.log(name, req.Params.Arguments, false, err.Error())
-		} else if result != nil && result.IsError {
-			errMsg := "tool error"
-			if len(result.Content) > 0 {
-				if tc, ok := result.Content[0].(*mcp.TextContent); ok {
-					errMsg = tc.Text
-				}
-			}
-			s.auditLog.log(name, req.Params.Arguments, false, errMsg)
-		} else {
-			s.auditLog.log(name, req.Params.Arguments, true, "")
-		}
-
-		return result, out, err
-	}
-
-	mcp.AddTool(s.mcpServer, tool, wrappedHandler)
-	s.toolNames = append(s.toolNames, name)
+	addToolWithMetaTyped[struct{}](s, name, description, meta, handler)
 }
 
 // --- MCP App registration entry points ---

internal/mcp/apps_html/insights.html

@@ -404,10 +404,17 @@ <h3>Top users</h3>
   var initialized = false;
   jcApp.onToolResult(function(params) {
     if (initialized) return;
-    initialized = true;
+    // Only mark initialized after a successful render; a thrown error or
+    // null payload leaves initialized=false so the setTimeout fallback
+    // below takes over and issues a real callTool with proper error
+    // handling. Previously setting the flag up-front could leave the UI
+    // stuck on the loading spinner.
     try {
       var data = jcApp.parseToolResult(params);
-      if (data) renderAll(data);
+      if (data) {
+        initialized = true;
+        renderAll(data);
+      }
     } catch (e) { /* proactive fetch below will retry */ }
   });