Fix chat crash React 409 (#1849)

docs/TOOLS.md

@@ -318,64 +318,26 @@ function updateMcpToolSetting(toolId: string, enabled: boolean) {
 3. **Settings UI**: `ToolSettingsSection` component reads from the registry to generate UI
 4. **Tool Execution**: `AutonomousAgentChainRunner.getAvailableTools()` filters tools based on settings
 
-### Tool Execution Flow
+## Tool Call Rendering Roots
 
-1. Agent calls `getAvailableTools()` which:
+React invariant #409 surfaced when tool-call banners attempted to render into React roots that had already been unmounted. To prevent this regression, the plugin routes all banner rendering through the shared manager in `src/components/chat-components/toolCallRootManager.tsx`.
 
-   - Gets enabled tool IDs from settings array (`autonomousAgentEnabledToolIds`)
-   - Calls `registry.getEnabledTools()` to get actual tool implementations
-   - Filters based on vault availability and user preferences
+### Manager Responsibilities
 
-2. Model adapter receives tool list and:
+- Tracks `{ root, isUnmounting }` per message/tool call via `window.__copilotToolCallRoots`.
+- `ensureToolCallRoot` finalises pending disposals and creates a new `createRoot` when needed.
+- `renderToolCallBanner` renders `<ToolCallBanner />` into the managed root; components never call `root.render` directly.
+- `removeToolCallRoot` and `cleanupMessageToolCallRoots` schedule unmounts on the next tick and drop entries only after disposal completes.
+- `cleanupStaleToolCallRoots` purges message IDs older than one hour to avoid leaking historical roots.
 
-   - Generates tool descriptions for the system prompt
-   - Includes tool-specific instructions based on enabled tools
+### Integration Notes
 
-3. When tool is called:
-   - XML parsing extracts tool name and parameters
-   - Tool is executed via its `func` implementation
-   - Results are formatted and returned to the agent
+`ChatSingleMessage` keeps `const rootsRef = useRef(getMessageToolCallRoots(messageId))`, which provides a stable registry for each message. The component delegates all lifecycle calls to the manager and snapshots `rootsRef.current` inside effect cleanup to satisfy `react-hooks/exhaustive-deps`.
 
-## Benefits of This Architecture
+### Verification
 
-1. **Modularity**: Each tool is self-contained with metadata
-2. **Extensibility**: New tools can be added without core changes
-3. **Backward Compatibility**: Settings structure preserved while supporting new tools
-4. **Dynamic UI**: Settings automatically adapt to registered tools
-5. **Smart Prompts**: System prompts include only relevant tool instructions
-6. **MCP Ready**: Architecture supports dynamic tool registration from external sources
+Run the focused test to cover the streaming behaviour and tool-call integration:
 
-## Testing Your Tools
-
-```typescript
-// Test tool registration
-const registry = ToolRegistry.getInstance();
-registry.clear();
-initializeBuiltinTools(vault);
-
-// Verify tool is registered
-const allTools = registry.getAllTools();
-console.log(
-  "Registered tools:",
-  allTools.map((t) => t.metadata.id)
-);
-
-// Test with settings
-const enabledIds = new Set(["myNewTool", "localSearch"]);
-const enabledTools = registry.getEnabledTools(enabledIds, true);
-console.log(
-  "Enabled tools:",
-  enabledTools.map((t) => t.name)
-);
 ```
-
-This architecture provides a clean, extensible foundation for the tool system while maintaining simplicity and backward compatibility.
-
-## Summary
-
-The tool system's layered approach allows for:
-
-- Clear, comprehensive tool documentation at the schema level
-- Model-agnostic instructions that work for most LLMs
-- Targeted model-specific adaptations when necessary
-- Easy extension for new tools without modifying core infrastructure
+npm test -- src/components/chat-components/ChatSingleMessage.test.tsx
+```

src/components/chat-components/ChatSingleMessage.tsx

@@ -1,5 +1,4 @@
 import { ChatButtons } from "@/components/chat-components/ChatButtons";
-import { ToolCallBanner } from "@/components/chat-components/ToolCallBanner";
 import { SourcesModal } from "@/components/modals/SourcesModal";
 import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip";
 import {
@@ -9,6 +8,15 @@ import {
   ContextFolderBadge,
 } from "@/components/chat-components/ContextBadges";
 import { InlineMessageEditor } from "@/components/chat-components/InlineMessageEditor";
+import {
+  cleanupMessageToolCallRoots,
+  cleanupStaleToolCallRoots,
+  ensureToolCallRoot,
+  getMessageToolCallRoots,
+  renderToolCallBanner,
+  removeToolCallRoot,
+  type ToolCallRootRecord,
+} from "@/components/chat-components/toolCallRootManager";
 import { USER_SENDER } from "@/constants";
 import { cn } from "@/lib/utils";
 import { parseToolCallMarkers } from "@/LLMProviders/chainRunner/utils/toolCallParser";
@@ -18,13 +26,6 @@ import { ChatMessage } from "@/types/message";
 import { cleanMessageForCopy, insertIntoEditor } from "@/utils";
 import { App, Component, MarkdownRenderer, MarkdownView, TFile } from "obsidian";
 import React, { useCallback, useEffect, useRef, useState } from "react";
-import ReactDOM, { Root } from "react-dom/client";
-
-declare global {
-  interface Window {
-    __copilotToolCallRoots?: Map<string, Map<string, Root>>;
-  }
-}
 
 const FOOTNOTE_SUFFIX_PATTERN = /^\d+-\d+$/;
 
@@ -151,22 +152,9 @@ const ChatSingleMessage: React.FC<ChatSingleMessageProps> = ({
   );
 
   // Store roots in a global map to preserve them across component instances
-  const getGlobalRootsMap = () => {
-    if (!window.__copilotToolCallRoots) {
-      window.__copilotToolCallRoots = new Map<string, Map<string, Root>>();
-    }
-    return window.__copilotToolCallRoots;
-  };
-
-  const getRootsForMessage = () => {
-    const globalMap = getGlobalRootsMap();
-    if (!globalMap.has(messageId.current)) {
-      globalMap.set(messageId.current, new Map<string, Root>());
-    }
-    return globalMap.get(messageId.current)!;
-  };
-
-  const rootsRef = useRef<Map<string, Root>>(getRootsForMessage());
+  const rootsRef = useRef<Map<string, ToolCallRootRecord>>(
+    getMessageToolCallRoots(messageId.current)
+  );
 
   const copyToClipboard = () => {
     if (!navigator.clipboard || !navigator.clipboard.writeText) {
@@ -383,26 +371,9 @@ const ChatSingleMessage: React.FC<ChatSingleMessageProps> = ({
             currentIndex++;
           } else if (segment.type === "toolCall" && segment.toolCall) {
             const toolCallId = segment.toolCall.id;
-            const existingDiv = document.getElementById(`tool-call-${toolCallId}`);
+            let container = document.getElementById(`tool-call-${toolCallId}`);
 
-            if (existingDiv) {
-              // Update existing tool call
-              const root = rootsRef.current.get(toolCallId);
-              if (root && !isUnmountingRef.current) {
-                root.render(
-                  <ToolCallBanner
-                    toolName={segment.toolCall.toolName}
-                    displayName={segment.toolCall.displayName}
-                    emoji={segment.toolCall.emoji}
-                    isExecuting={segment.toolCall.isExecuting}
-                    result={segment.toolCall.result || null}
-                    confirmationMessage={segment.toolCall.confirmationMessage}
-                  />
-                );
-              }
-              currentIndex++;
-            } else {
-              // Create new tool call
+            if (!container) {
               const insertBefore = contentRef.current!.children[currentIndex];
               const toolDiv = document.createElement("div");
               toolDiv.className = "tool-call-container";
@@ -414,23 +385,22 @@ const ChatSingleMessage: React.FC<ChatSingleMessageProps> = ({
                 contentRef.current!.appendChild(toolDiv);
               }
 
-              const root = ReactDOM.createRoot(toolDiv);
-              rootsRef.current.set(toolCallId, root);
-
-              if (!isUnmountingRef.current) {
-                root.render(
-                  <ToolCallBanner
-                    toolName={segment.toolCall.toolName}
-                    displayName={segment.toolCall.displayName}
-                    emoji={segment.toolCall.emoji}
-                    isExecuting={segment.toolCall.isExecuting}
-                    result={segment.toolCall.result || null}
-                    confirmationMessage={segment.toolCall.confirmationMessage}
-                  />
-                );
-              }
-              currentIndex++;
+              container = toolDiv;
             }
+
+            const rootRecord = ensureToolCallRoot(
+              messageId.current,
+              rootsRef.current,
+              toolCallId,
+              container as HTMLElement,
+              "render refresh"
+            );
+
+            if (!isUnmountingRef.current && !rootRecord.isUnmounting) {
+              renderToolCallBanner(rootRecord, segment.toolCall);
+            }
+
+            currentIndex++;
           }
         });
 
@@ -445,18 +415,7 @@ const ChatSingleMessage: React.FC<ChatSingleMessageProps> = ({
           if (!currentToolCallIds.has(id)) {
             const element = document.getElementById(`tool-call-${id}`);
             if (element) {
-              const root = rootsRef.current.get(id);
-              if (root) {
-                // Defer unmounting to avoid React rendering conflicts
-                setTimeout(() => {
-                  try {
-                    root.unmount();
-                  } catch (error) {
-                    console.debug("Error unmounting tool call root:", error);
-                  }
-                  rootsRef.current.delete(id);
-                }, 0);
-              }
+              removeToolCallRoot(messageId.current, rootsRef.current, id, "tool call removal");
               element.remove();
             }
           }
@@ -474,29 +433,11 @@ const ChatSingleMessage: React.FC<ChatSingleMessageProps> = ({
   useEffect(() => {
     const currentComponentRef = componentRef;
     const currentMessageId = messageId.current;
+    const messageRootsSnapshot = rootsRef.current;
 
     // Clean up old message roots to prevent memory leaks (older than 1 hour)
     const cleanupOldRoots = () => {
-      const globalMap = getGlobalRootsMap();
-      const oneHourAgo = Date.now() - 60 * 60 * 1000;
-
-      globalMap.forEach((roots, msgId) => {
-        // Extract timestamp from message ID if it's in epoch format
-        const timestamp = parseInt(msgId);
-        if (!isNaN(timestamp) && timestamp < oneHourAgo) {
-          // Defer cleanup to avoid React rendering conflicts
-          setTimeout(() => {
-            roots.forEach((root) => {
-              try {
-                root.unmount();
-              } catch {
-                // Ignore errors
-              }
-            });
-            globalMap.delete(msgId);
-          }, 0);
-        }
-      });
+      cleanupStaleToolCallRoots();
     };
 
     // Run cleanup on mount
@@ -517,20 +458,7 @@ const ChatSingleMessage: React.FC<ChatSingleMessageProps> = ({
         // Only clean up roots if this is a temporary message (streaming message)
         // Permanent messages keep their roots to preserve tool call banners
         if (currentMessageId.startsWith("temp-")) {
-          const globalMap = getGlobalRootsMap();
-          const messageRoots = globalMap.get(currentMessageId);
-
-          if (messageRoots) {
-            messageRoots.forEach((root) => {
-              try {
-                root.unmount();
-              } catch (error) {
-                // Ignore unmount errors during cleanup
-                console.debug("Error unmounting React root during cleanup:", error);
-              }
-            });
-            globalMap.delete(currentMessageId);
-          }
+          cleanupMessageToolCallRoots(currentMessageId, messageRootsSnapshot, "component cleanup");
         }
       }, 0);
     };

src/components/chat-components/toolCallRootManager.tsx

@@ -0,0 +1,231 @@
+import React from "react";
+import { createRoot, Root } from "react-dom/client";
+
+import { ToolCallBanner } from "@/components/chat-components/ToolCallBanner";
+import type { ToolCallMarker } from "@/LLMProviders/chainRunner/utils/toolCallParser";
+import { logWarn } from "@/logger";
+
+declare global {
+  interface Window {
+    __copilotToolCallRoots?: Map<string, Map<string, ToolCallRootRecord>>;
+  }
+}
+
+export interface ToolCallRootRecord {
+  root: Root;
+  isUnmounting: boolean;
+}
+
+const STALE_ROOT_MAX_AGE_MS = 60 * 60 * 1000;
+
+/**
+ * Retrieve the global registry that keeps track of tool call React roots.
+ * The registry is stored on `window` to preserve state across component lifecycles.
+ */
+const getRegistry = (): Map<string, Map<string, ToolCallRootRecord>> => {
+  if (!window.__copilotToolCallRoots) {
+    window.__copilotToolCallRoots = new Map<string, Map<string, ToolCallRootRecord>>();
+  }
+
+  return window.__copilotToolCallRoots;
+};
+
+/**
+ * Remove the message entry from the registry when it no longer has active tool call roots.
+ */
+const pruneEmptyMessageEntry = (
+  messageId: string,
+  messageRoots: Map<string, ToolCallRootRecord>
+): void => {
+  if (messageRoots.size > 0) {
+    return;
+  }
+
+  const registry = getRegistry();
+  const currentRoots = registry.get(messageId);
+
+  if (currentRoots === messageRoots) {
+    registry.delete(messageId);
+  }
+};
+
+/**
+ * Unmount a tool call root, mark it as inactive, and remove it from the registry.
+ */
+const disposeToolCallRoot = (
+  messageId: string,
+  messageRoots: Map<string, ToolCallRootRecord>,
+  toolCallId: string,
+  record: ToolCallRootRecord,
+  logContext: string
+): void => {
+  try {
+    record.root.unmount();
+  } catch (error) {
+    logWarn(`Error unmounting tool call root during ${logContext}`, toolCallId, error);
+  }
+
+  record.isUnmounting = false;
+
+  if (messageRoots.get(toolCallId) === record) {
+    messageRoots.delete(toolCallId);
+  }
+
+  pruneEmptyMessageEntry(messageId, messageRoots);
+};
+
+/**
+ * Schedule a deferred unmount for a tool call root while preventing duplicate requests.
+ */
+const scheduleToolCallRootDisposal = (
+  messageId: string,
+  messageRoots: Map<string, ToolCallRootRecord>,
+  toolCallId: string,
+  record: ToolCallRootRecord,
+  logContext: string
+): void => {
+  if (record.isUnmounting) {
+    return;
+  }
+
+  record.isUnmounting = true;
+
+  setTimeout(() => {
+    const registry = getRegistry();
+    const currentRoots = registry.get(messageId);
+    const currentRecord = currentRoots?.get(toolCallId);
+
+    if (!currentRoots || currentRecord !== record) {
+      record.isUnmounting = false;
+      pruneEmptyMessageEntry(messageId, messageRoots);
+      return;
+    }
+
+    disposeToolCallRoot(messageId, currentRoots, toolCallId, currentRecord, logContext);
+  }, 0);
+};
+
+/**
+ * Ensure a React root exists for the provided tool call container and return the root record.
+ */
+export const ensureToolCallRoot = (
+  messageId: string,
+  messageRoots: Map<string, ToolCallRootRecord>,
+  toolCallId: string,
+  container: HTMLElement,
+  logContext: string
+): ToolCallRootRecord => {
+  let record = messageRoots.get(toolCallId);
+
+  if (record?.isUnmounting) {
+    disposeToolCallRoot(
+      messageId,
+      messageRoots,
+      toolCallId,
+      record,
+      `${logContext} (finalizing stale root)`
+    );
+    record = undefined;
+  }
+
+  if (!record) {
+    record = {
+      root: createRoot(container),
+      isUnmounting: false,
+    };
+
+    messageRoots.set(toolCallId, record);
+  }
+
+  return record;
+};
+
+/**
+ * Render the `ToolCallBanner` component into the provided root record.
+ */
+export const renderToolCallBanner = (
+  record: ToolCallRootRecord,
+  toolCall: ToolCallMarker
+): void => {
+  record.root.render(
+    <ToolCallBanner
+      toolName={toolCall.toolName}
+      displayName={toolCall.displayName}
+      emoji={toolCall.emoji}
+      isExecuting={toolCall.isExecuting}
+      result={toolCall.result || null}
+      confirmationMessage={toolCall.confirmationMessage}
+    />
+  );
+};
+
+/**
+ * Schedule the removal of a tool call root from a message root collection.
+ */
+export const removeToolCallRoot = (
+  messageId: string,
+  messageRoots: Map<string, ToolCallRootRecord>,
+  toolCallId: string,
+  logContext: string
+): void => {
+  const record = messageRoots.get(toolCallId);
+
+  if (!record) {
+    return;
+  }
+
+  scheduleToolCallRootDisposal(messageId, messageRoots, toolCallId, record, logContext);
+};
+
+/**
+ * Return (and create if necessary) the tool call root map for a specific message.
+ */
+export const getMessageToolCallRoots = (messageId: string): Map<string, ToolCallRootRecord> => {
+  const registry = getRegistry();
+  let messageRoots = registry.get(messageId);
+
+  if (!messageRoots) {
+    messageRoots = new Map<string, ToolCallRootRecord>();
+    registry.set(messageId, messageRoots);
+  }
+
+  return messageRoots;
+};
+
+/**
+ * Clean up tool call roots for messages whose identifiers encode timestamps older than the configured threshold.
+ */
+export const cleanupStaleToolCallRoots = (now: number = Date.now()): void => {
+  const registry = getRegistry();
+
+  registry.forEach((messageRoots, messageId) => {
+    const timestamp = Number.parseInt(messageId, 10);
+
+    if (Number.isNaN(timestamp) || now - timestamp < STALE_ROOT_MAX_AGE_MS) {
+      return;
+    }
+
+    messageRoots.forEach((record, toolCallId) => {
+      scheduleToolCallRootDisposal(
+        messageId,
+        messageRoots,
+        toolCallId,
+        record,
+        "stale message cleanup"
+      );
+    });
+  });
+};
+
+/**
+ * Schedule cleanup for all tool call roots owned by a specific message.
+ */
+export const cleanupMessageToolCallRoots = (
+  messageId: string,
+  messageRoots: Map<string, ToolCallRootRecord>,
+  logContext: string
+): void => {
+  messageRoots.forEach((record, toolCallId) => {
+    scheduleToolCallRootDisposal(messageId, messageRoots, toolCallId, record, logContext);
+  });
+};