fix: resolve tool scheduling, cancellation, and search issues

packages/cli/src/ui/hooks/useHistoryManager.ts

@@ -13,6 +13,10 @@ import {
   DEFAULT_HISTORY_MAX_ITEMS,
 } from '../../constants/historyLimits.js';
 
+// Global counter for generating unique message IDs across all hook instances.
+// This prevents ID collisions when multiple useHistory hooks use the same baseTimestamp.
+let globalMessageIdCounter = 0;
+
 // Type for the updater function passed to updateHistoryItem
 type HistoryItemUpdater = (
   prevItem: HistoryItem,
@@ -49,7 +53,6 @@ export function useHistory(
   options?: UseHistoryOptions,
 ): UseHistoryManagerReturn {
   const [history, setHistory] = useState<HistoryItem[]>([]);
-  const messageIdCounterRef = useRef(0);
   const maxItems = options?.maxItems;
   const maxBytes = options?.maxBytes;
   const limits = useMemo(
@@ -63,10 +66,11 @@ export function useHistory(
     setHistory((prev) => trimHistory(prev, limits));
   }, [limits]);
 
-  // Generates a unique message ID based on a timestamp and a counter.
+  // Generates a unique message ID based on a timestamp and a global counter.
+  // Using a global counter ensures uniqueness across all hook instances.
   const getNextMessageId = useCallback((baseTimestamp: number): number => {
-    messageIdCounterRef.current += 1;
-    return baseTimestamp + messageIdCounterRef.current;
+    globalMessageIdCounter += 1;
+    return baseTimestamp * 1000 + globalMessageIdCounter;
   }, []);
 
   const loadHistory = useCallback((newHistory: HistoryItem[]) => {
@@ -128,10 +132,10 @@ export function useHistory(
     [],
   );
 
-  // Clears the entire history state and resets the ID counter.
+  // Clears the entire history state. Note: we do NOT reset the global counter
+  // to ensure IDs remain unique across conversation clears within the same session.
   const clearItems = useCallback(() => {
     setHistory([]);
-    messageIdCounterRef.current = 0;
     ConversationContext.startNewConversation();
   }, []);
 

packages/core/src/core/coreToolScheduler.ts

@@ -412,6 +412,9 @@ export class CoreToolScheduler {
   > = new Map();
   private nextPublishIndex = 0;
   private readonly toolContextInteractiveMode: boolean;
+  // Track the abort signal for each tool call so we can use it when handling
+  // confirmation responses from the message bus
+  private callIdToSignal: Map<string, AbortSignal> = new Map();
 
   constructor(options: CoreToolSchedulerOptions) {
     this.config = options.config;
@@ -491,12 +494,20 @@ export class CoreToolScheduler {
           : ToolConfirmationOutcome.Cancel
         : ToolConfirmationOutcome.Cancel);
 
-    const abortController = new AbortController();
+    // Use the original signal stored for this call, or create a new one as fallback
+    const originalSignal = this.callIdToSignal.get(callId);
+    if (!originalSignal && toolSchedulerLogger.enabled) {
+      toolSchedulerLogger.debug(
+        () =>
+          `Using fallback AbortSignal for callId=${callId} (original signal not found in map)`,
+      );
+    }
+    const signal = originalSignal ?? new AbortController().signal;
     void this.handleConfirmationResponse(
       callId,
       waitingToolCall.confirmationDetails.onConfirm,
       derivedOutcome,
-      abortController.signal,
+      signal,
       response.payload,
       true,
     );
@@ -940,6 +951,8 @@ export class CoreToolScheduler {
         }
 
         const { request: reqInfo, invocation } = toolCall;
+        // Store the signal for this call so we can use it later in message bus responses
+        this.callIdToSignal.set(reqInfo.callId, signal);
 
         try {
           if (signal.aborted) {
@@ -1352,37 +1365,51 @@ export class CoreToolScheduler {
     });
   }
 
+  // Reentrancy guard for publishBufferedResults to prevent race conditions
+  // when multiple async tool completions trigger publishing simultaneously
+  private isPublishingBufferedResults = false;
+
   private async publishBufferedResults(signal: AbortSignal): Promise<void> {
-    const callsInOrder = this.toolCalls.filter(
-      (call) => call.status === 'scheduled' || call.status === 'executing',
-    );
+    // Prevent reentrant calls which can cause race conditions
+    if (this.isPublishingBufferedResults) {
+      return;
+    }
+    this.isPublishingBufferedResults = true;
 
-    // Publish results in original request order
-    while (this.nextPublishIndex < callsInOrder.length) {
-      const expectedCall = callsInOrder[this.nextPublishIndex];
-      const buffered = this.pendingResults.get(expectedCall.request.callId);
+    try {
+      const callsInOrder = this.toolCalls.filter(
+        (call) => call.status === 'scheduled' || call.status === 'executing',
+      );
 
-      if (!buffered) {
-        // Next result not ready yet, stop publishing
-        break;
-      }
+      // Publish results in original request order
+      while (this.nextPublishIndex < callsInOrder.length) {
+        const expectedCall = callsInOrder[this.nextPublishIndex];
+        const buffered = this.pendingResults.get(expectedCall.request.callId);
 
-      // Publish this result
-      await this.publishResult(buffered, signal);
+        if (!buffered) {
+          // Next result not ready yet, stop publishing
+          break;
+        }
 
-      // Remove from buffer
-      this.pendingResults.delete(buffered.callId);
-      this.nextPublishIndex++;
-    }
+        // Publish this result
+        await this.publishResult(buffered, signal);
 
-    // Check if all tools completed
-    if (
-      this.nextPublishIndex === callsInOrder.length &&
-      callsInOrder.length > 0
-    ) {
-      // Reset for next batch
-      this.nextPublishIndex = 0;
-      this.pendingResults.clear();
+        // Remove from buffer
+        this.pendingResults.delete(buffered.callId);
+        this.nextPublishIndex++;
+      }
+
+      // Check if all tools completed
+      if (
+        this.nextPublishIndex === callsInOrder.length &&
+        callsInOrder.length > 0
+      ) {
+        // Reset for next batch
+        this.nextPublishIndex = 0;
+        this.pendingResults.clear();
+      }
+    } finally {
+      this.isPublishingBufferedResults = false;
     }
   }
 
@@ -1548,7 +1575,9 @@ export class CoreToolScheduler {
       const completedCalls = [...this.toolCalls] as CompletedToolCall[];
       this.toolCalls = [];
 
+      // Clean up signal mappings for completed calls
       for (const call of completedCalls) {
+        this.callIdToSignal.delete(call.request.callId);
         logToolCall(this.config, new ToolCallEvent(call));
       }
 

packages/core/src/tools/grep.ts

@@ -29,6 +29,20 @@ import {
 } from '../utils/toolOutputLimiter.js';
 import { MessageBus } from '../confirmation-bus/message-bus.js';
 
+/**
+ * Checks if a glob pattern contains brace expansion syntax that git grep doesn't support.
+ * Git grep pathspecs don't support shell-style brace expansion like {ts,tsx,js}.
+ * Uses indexOf for O(n) complexity instead of regex to avoid ReDoS vulnerability.
+ */
+function hasBraceExpansion(pattern: string): boolean {
+  const braceStart = pattern.indexOf('{');
+  if (braceStart === -1) return false;
+  const braceEnd = pattern.indexOf('}', braceStart);
+  if (braceEnd === -1) return false;
+  const commaPos = pattern.indexOf(',', braceStart);
+  return commaPos !== -1 && commaPos < braceEnd;
+}
+
 // --- Interfaces ---
 
 /**
@@ -500,7 +514,10 @@ class GrepToolInvocation extends BaseToolInvocation<
 
     try {
       // --- Strategy 1: git grep ---
-      const isGit = isGitRepository(absolutePath);
+      // Skip git grep if include pattern has brace expansion (e.g., *.{ts,tsx})
+      // because git grep pathspecs don't support shell-style brace expansion.
+      const hasBracePattern = include && hasBraceExpansion(include);
+      const isGit = !hasBracePattern && isGitRepository(absolutePath);
       const gitAvailable = isGit && (await this.isCommandAvailable('git'));
 
       if (gitAvailable) {
@@ -526,12 +543,23 @@ class GrepToolInvocation extends BaseToolInvocation<
             const stdoutChunks: Buffer[] = [];
             const stderrChunks: Buffer[] = [];
 
+            // Handle abort signal to kill child process
+            const abortHandler = () => {
+              if (!child.killed) {
+                child.kill('SIGTERM');
+              }
+              reject(new Error('git grep aborted'));
+            };
+            options.signal.addEventListener('abort', abortHandler);
+
             child.stdout.on('data', (chunk) => stdoutChunks.push(chunk));
             child.stderr.on('data', (chunk) => stderrChunks.push(chunk));
-            child.on('error', (err) =>
-              reject(new Error(`Failed to start git grep: ${err.message}`)),
-            );
+            child.on('error', (err) => {
+              options.signal.removeEventListener('abort', abortHandler);
+              reject(new Error(`Failed to start git grep: ${err.message}`));
+            });
             child.on('close', (code) => {
+              options.signal.removeEventListener('abort', abortHandler);
               const stdoutData = Buffer.concat(stdoutChunks).toString('utf8');
               const stderrData = Buffer.concat(stderrChunks).toString('utf8');
               if (code === 0) resolve(stdoutData);
@@ -596,6 +624,16 @@ class GrepToolInvocation extends BaseToolInvocation<
             const stdoutChunks: Buffer[] = [];
             const stderrChunks: Buffer[] = [];
 
+            // Handle abort signal to kill child process
+            const abortHandler = () => {
+              if (!child.killed) {
+                child.kill('SIGTERM');
+              }
+              cleanup();
+              reject(new Error('system grep aborted'));
+            };
+            options.signal.addEventListener('abort', abortHandler);
+
             const onData = (chunk: Buffer) => stdoutChunks.push(chunk);
             const onStderr = (chunk: Buffer) => {
               const stderrStr = chunk.toString();
@@ -632,6 +670,7 @@ class GrepToolInvocation extends BaseToolInvocation<
             };
 
             const cleanup = () => {
+              options.signal.removeEventListener('abort', abortHandler);
               child.stdout.removeListener('data', onData);
               child.stderr.removeListener('data', onStderr);
               child.removeListener('error', onError);

packages/core/src/tools/tool-registry.ts

@@ -70,7 +70,7 @@ Signal: Signal number or \`(none)\` if no signal was received.
 
   async execute(
     params: ToolParams,
-    _signal: AbortSignal,
+    signal: AbortSignal,
     _updateOutput?: (output: string) => void,
   ): Promise<ToolResult> {
     const callCommand = this.config.getToolCallCommand()!;
@@ -82,55 +82,67 @@ Signal: Signal number or \`(none)\` if no signal was received.
     let stderr = '';
     let error: Error | null = null;
     let code: number | null = null;
-    let signal: NodeJS.Signals | null = null;
-
-    await new Promise<void>((resolve) => {
-      const onStdout = (data: Buffer) => {
-        stdout += data?.toString();
-      };
-
-      const onStderr = (data: Buffer) => {
-        stderr += data?.toString();
-      };
+    let exitSignal: NodeJS.Signals | null = null;
 
-      const onError = (err: Error) => {
-        error = err;
-      };
-
-      const onClose = (
-        _code: number | null,
-        _signal: NodeJS.Signals | null,
-      ) => {
-        code = _code;
-        signal = _signal;
-        cleanup();
-        resolve();
-      };
+    // Handle abort signal to kill the child process
+    const abortHandler = () => {
+      if (!child.killed) {
+        child.kill('SIGTERM');
+      }
+    };
+    signal.addEventListener('abort', abortHandler);
 
-      const cleanup = () => {
-        child.stdout.removeListener('data', onStdout);
-        child.stderr.removeListener('data', onStderr);
-        child.removeListener('error', onError);
-        child.removeListener('close', onClose);
-        if (child.connected) {
-          child.disconnect();
-        }
-      };
+    try {
+      await new Promise<void>((resolve) => {
+        const onStdout = (data: Buffer) => {
+          stdout += data?.toString();
+        };
+
+        const onStderr = (data: Buffer) => {
+          stderr += data?.toString();
+        };
+
+        const onError = (err: Error) => {
+          error = err;
+        };
+
+        const onClose = (
+          _code: number | null,
+          _signal: NodeJS.Signals | null,
+        ) => {
+          code = _code;
+          exitSignal = _signal;
+          cleanup();
+          resolve();
+        };
+
+        const cleanup = () => {
+          child.stdout.removeListener('data', onStdout);
+          child.stderr.removeListener('data', onStderr);
+          child.removeListener('error', onError);
+          child.removeListener('close', onClose);
+          if (child.connected) {
+            child.disconnect();
+          }
+        };
 
-      child.stdout.on('data', onStdout);
-      child.stderr.on('data', onStderr);
-      child.on('error', onError);
-      child.on('close', onClose);
-    });
+        child.stdout.on('data', onStdout);
+        child.stderr.on('data', onStderr);
+        child.on('error', onError);
+        child.on('close', onClose);
+      });
+    } finally {
+      signal.removeEventListener('abort', abortHandler);
+    }
 
     // if there is any error, non-zero exit code, signal, or stderr, return error details instead of stdout
-    if (error || code !== 0 || signal || stderr) {
+    if (error || code !== 0 || exitSignal || stderr) {
       const llmContent = [
         `Stdout: ${stdout || '(empty)'}`,
         `Stderr: ${stderr || '(empty)'}`,
         `Error: ${error ?? '(none)'}`,
         `Exit Code: ${code ?? '(none)'}`,
-        `Signal: ${signal ?? '(none)'}`,
+        `Signal: ${exitSignal ?? '(none)'}`,
       ].join('\n');
       return {
         llmContent,