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/cli/src/ui/hooks/useReactToolScheduler.ts

@@ -310,7 +310,16 @@ export function useReactToolScheduler(
         ? request.map(ensureAgentId)
         : ensureAgentId(request);
 
-      scheduler.schedule(normalizedRequest, signal);
+      // The scheduler.schedule() returns a Promise that rejects when the abort
+      // signal fires while tool calls are queued. We intentionally catch and
+      // ignore these rejections because:
+      // 1. Cancellation is an expected user action, not an error
+      // 2. The UI state is updated via cancelAllToolCalls() synchronously
+      // 3. Tool results are not needed after cancellation
+      scheduler.schedule(normalizedRequest, signal).catch(() => {
+        // Silently ignore cancellation rejections - this is expected behavior
+        // when the user presses ESC to cancel queued tool calls
+      });
     },
     [scheduler],
   );

packages/core/src/core/coreToolScheduler.test.ts

@@ -2254,6 +2254,246 @@ describe('CoreToolScheduler Buffered Parallel Execution', () => {
     // Verify ordered publishing despite error in tool 2
     expect(publishOrder).toEqual([1, 2, 3]); // Request order maintained
   });
+
+  it('should handle race condition when later tools complete while publishBufferedResults is exiting', async () => {
+    // This test exercises the race condition where:
+    // 1. Tool #3 finishes first, calls publishBufferedResults
+    // 2. publishBufferedResults waits for tool #1, breaks out of inner while loop
+    // 3. Just as it checks pendingPublishRequest (false) and is about to exit do-while
+    // 4. Tool #1 finishes, sets pendingPublishRequest=true, returns immediately
+    // 5. Without the fix: first publishBufferedResults exits without processing buffered results
+    // 6. With the fix: the finally block detects pendingResults.size > 0 and reschedules
+    //
+    // The fix adds a check in the finally block to reschedule if pendingResults.size > 0
+
+    const completionOrder: number[] = [];
+    const publishOrder: number[] = [];
+
+    // Use a deferred promise pattern to precisely control timing
+    const resolvers: Map<number, () => void> = new Map();
+
+    const executeFn = vi
+      .fn()
+      .mockImplementation(async (args: { call: number }) => {
+        const callNum = args.call;
+        // Create a promise that we can resolve externally
+        await new Promise<void>((resolve) => {
+          resolvers.set(callNum, resolve);
+        });
+        completionOrder.push(callNum);
+        return { llmContent: `Call ${callNum} done` };
+      });
+
+    const mockTool = new MockTool({ name: 'mockTool', execute: executeFn });
+    const mockToolRegistry = {
+      getTool: () => mockTool,
+      getFunctionDeclarations: () => [],
+      tools: new Map(),
+      discovery: {},
+      registerTool: () => {},
+      getToolByName: () => mockTool,
+      getToolByDisplayName: () => mockTool,
+      getTools: () => [],
+      discoverTools: async () => {},
+      getAllTools: () => [],
+      getToolsByServer: () => [],
+    } as unknown as ToolRegistry;
+
+    const onToolCallsUpdate = vi.fn();
+    const mockPolicyEngine = createMockPolicyEngine();
+
+    const mockConfig = {
+      getSessionId: () => 'test-session-id',
+      getUsageStatisticsEnabled: () => true,
+      getDebugMode: () => false,
+      getApprovalMode: () => ApprovalMode.YOLO,
+      getEphemeralSettings: () => ({}),
+      getAllowedTools: () => [],
+      getContentGeneratorConfig: () => ({
+        model: 'test-model',
+        authType: 'oauth-personal',
+      }),
+      getToolRegistry: () => mockToolRegistry,
+      getMessageBus: vi.fn().mockReturnValue(createMockMessageBus()),
+      getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
+    } as unknown as Config;
+
+    const scheduler = new CoreToolScheduler({
+      config: mockConfig,
+      onAllToolCallsComplete: vi.fn(),
+      onToolCallsUpdate: (calls) => {
+        onToolCallsUpdate(calls);
+        calls.forEach((call) => {
+          if (call.status === 'success') {
+            const callNum = (call.request.args as { call: number }).call;
+            if (!publishOrder.includes(callNum)) {
+              publishOrder.push(callNum);
+            }
+          }
+        });
+      },
+      getPreferredEditor: () => 'vscode',
+      onEditorClose: vi.fn(),
+    });
+
+    const signal = new AbortController().signal;
+
+    // Schedule 5 tool calls (simulating the scenario from the bug report)
+    await scheduler.schedule(
+      [1, 2, 3, 4, 5].map((n) => ({
+        callId: `call${n}`,
+        name: 'mockTool',
+        args: { call: n },
+        isClientInitiated: false,
+        prompt_id: 'test',
+      })),
+      signal,
+    );
+
+    // Wait for all tools to start executing and set up their resolvers
+    await vi.waitFor(
+      () => {
+        expect(resolvers.size).toBe(5);
+      },
+      { timeout: 1000 },
+    );
+
+    // Now complete tools in a specific order that triggers the race condition:
+    // Complete tool 3 first (middle of the batch)
+    resolvers.get(3)?.();
+
+    // Small delay to let publishBufferedResults start and break out waiting for tool 1
+    await new Promise((resolve) => setTimeout(resolve, 5));
+
+    // Complete tools 4 and 5
+    resolvers.get(4)?.();
+    resolvers.get(5)?.();
+
+    // Small delay
+    await new Promise((resolve) => setTimeout(resolve, 5));
+
+    // Complete tool 2
+    resolvers.get(2)?.();
+
+    // Small delay
+    await new Promise((resolve) => setTimeout(resolve, 5));
+
+    // Finally complete tool 1 (the blocker)
+    resolvers.get(1)?.();
+
+    // Wait for all calls to complete
+    await vi.waitFor(
+      () => {
+        expect(completionOrder.length).toBe(5);
+        expect(publishOrder.length).toBe(5);
+      },
+      { timeout: 2000 },
+    );
+
+    // Verify that despite the out-of-order completion, all results were published
+    // and in the correct request order
+    expect(publishOrder).toEqual([1, 2, 3, 4, 5]);
+  });
+
+  it('should recover when all later tools complete before first tool', async () => {
+    // Edge case: All tools except the first one complete, then the first one completes.
+    // Without the fix, the buffered results might get stuck.
+    const completionOrder: number[] = [];
+    const publishOrder: number[] = [];
+
+    const executeFn = vi
+      .fn()
+      .mockImplementation(async (args: { call: number }) => {
+        // First tool takes longest, all others complete quickly
+        if (args.call === 1) {
+          await new Promise((resolve) => setTimeout(resolve, 80));
+        } else {
+          // All other tools complete almost immediately but staggered
+          await new Promise((resolve) => setTimeout(resolve, args.call * 5));
+        }
+        completionOrder.push(args.call);
+        return { llmContent: `Call ${args.call} done` };
+      });
+
+    const mockTool = new MockTool({ name: 'mockTool', execute: executeFn });
+    const mockToolRegistry = {
+      getTool: () => mockTool,
+      getFunctionDeclarations: () => [],
+      tools: new Map(),
+      discovery: {},
+      registerTool: () => {},
+      getToolByName: () => mockTool,
+      getToolByDisplayName: () => mockTool,
+      getTools: () => [],
+      discoverTools: async () => {},
+      getAllTools: () => [],
+      getToolsByServer: () => [],
+    } as unknown as ToolRegistry;
+
+    const mockPolicyEngine = createMockPolicyEngine();
+
+    const mockConfig = {
+      getSessionId: () => 'test-session-id',
+      getUsageStatisticsEnabled: () => true,
+      getDebugMode: () => false,
+      getApprovalMode: () => ApprovalMode.YOLO,
+      getEphemeralSettings: () => ({}),
+      getAllowedTools: () => [],
+      getContentGeneratorConfig: () => ({
+        model: 'test-model',
+        authType: 'oauth-personal',
+      }),
+      getToolRegistry: () => mockToolRegistry,
+      getMessageBus: vi.fn().mockReturnValue(createMockMessageBus()),
+      getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine),
+    } as unknown as Config;
+
+    const scheduler = new CoreToolScheduler({
+      config: mockConfig,
+      onAllToolCallsComplete: vi.fn(),
+      onToolCallsUpdate: (calls) => {
+        calls.forEach((call) => {
+          if (call.status === 'success') {
+            const callNum = (call.request.args as { call: number }).call;
+            if (!publishOrder.includes(callNum)) {
+              publishOrder.push(callNum);
+            }
+          }
+        });
+      },
+      getPreferredEditor: () => 'vscode',
+      onEditorClose: vi.fn(),
+    });
+
+    const signal = new AbortController().signal;
+
+    // Schedule 5 tool calls
+    await scheduler.schedule(
+      [1, 2, 3, 4, 5].map((n) => ({
+        callId: `call${n}`,
+        name: 'mockTool',
+        args: { call: n },
+        isClientInitiated: false,
+        prompt_id: 'test',
+      })),
+      signal,
+    );
+
+    // Wait for all calls to complete
+    await vi.waitFor(
+      () => {
+        expect(completionOrder.length).toBe(5);
+        expect(publishOrder.length).toBe(5);
+      },
+      { timeout: 2000 },
+    );
+
+    // Completion order: 2, 3, 4, 5, 1 (first is slowest)
+    expect(completionOrder).toEqual([2, 3, 4, 5, 1]);
+
+    // But publish order should still be in request order
+    expect(publishOrder).toEqual([1, 2, 3, 4, 5]);
+  });
 });
 
 it('injects agentId into ContextAwareTool context', async () => {