Skip to content
Merged
157 changes: 143 additions & 14 deletions packages/core/src/core/coreToolScheduler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import type { Mock } from 'vitest';
import type {
Config,
Expand Down Expand Up @@ -2992,6 +2992,19 @@ describe('Fire hook functions integration', () => {
});

describe('Concurrent agent tool execution', () => {
Comment thread
wenshao marked this conversation as resolved.
Resolved
// Ensure tests are deterministic regardless of environment.
const origEnv = process.env['QWEN_CODE_MAX_TOOL_CONCURRENCY'];
beforeEach(() => {
delete process.env['QWEN_CODE_MAX_TOOL_CONCURRENCY'];
});
afterEach(() => {
if (origEnv !== undefined) {
process.env['QWEN_CODE_MAX_TOOL_CONCURRENCY'] = origEnv;
} else {
delete process.env['QWEN_CODE_MAX_TOOL_CONCURRENCY'];
}
});

function createScheduler(
tools: Map<string, MockTool>,
onAllToolCallsComplete: Mock,
Expand Down Expand Up @@ -3119,7 +3132,7 @@ describe('Fire hook functions integration', () => {
expect(startIndices.every((i) => i < firstEnd)).toBe(true);
});

it('should run agent tools concurrently while other tools run sequentially', async () => {
it('should run concurrency-safe tools in parallel and unsafe tools sequentially', async () => {
const executionLog: string[] = [];

const agentTool = new MockTool({
Expand All @@ -3138,10 +3151,11 @@ describe('Fire hook functions integration', () => {

const readTool = new MockTool({
name: 'read_file',
kind: Kind.Read,
execute: async (params) => {
const id = (params as { id: string }).id;
executionLog.push(`read:start:${id}`);
await new Promise((r) => setTimeout(r, 20));
await new Promise((r) => setTimeout(r, 50));
executionLog.push(`read:end:${id}`);
return {
llmContent: `Read ${id} done`,
Expand All @@ -3163,6 +3177,8 @@ describe('Fire hook functions integration', () => {
);

const abortController = new AbortController();
// All 4 calls are concurrency-safe (read_file=Kind.Read, agent=Agent name)
// so they form one parallel batch and all run concurrently.
const requests = [
{
Comment thread
wenshao marked this conversation as resolved.
Resolved
callId: '1',
Expand Down Expand Up @@ -3202,20 +3218,133 @@ describe('Fire hook functions integration', () => {
expect(completedCalls).toHaveLength(4);
expect(completedCalls.every((c) => c.status === 'success')).toBe(true);

// Non-agent tools should execute sequentially: read:1 finishes before read:2 starts
const read1End = executionLog.indexOf('read:end:1');
const read2Start = executionLog.indexOf('read:start:2');
expect(read1End).toBeLessThan(read2Start);

// Agent tools should execute concurrently: both start before either ends
const agentAStart = executionLog.indexOf('agent:start:A');
const agentBStart = executionLog.indexOf('agent:start:B');
const firstAgentEnd = Math.min(
// All 4 tools are concurrency-safe → they should all start
// before any of them finishes (parallel execution).
const allStarts = [
executionLog.indexOf('read:start:1'),
executionLog.indexOf('agent:start:A'),
executionLog.indexOf('read:start:2'),
executionLog.indexOf('agent:start:B'),
];
const firstEnd = Math.min(
executionLog.indexOf('read:end:1'),
executionLog.indexOf('agent:end:A'),
Comment thread
wenshao marked this conversation as resolved.
Resolved
executionLog.indexOf('read:end:2'),
executionLog.indexOf('agent:end:B'),
);
expect(agentAStart).toBeLessThan(firstAgentEnd);
expect(agentBStart).toBeLessThan(firstAgentEnd);
for (const start of allStarts) {
expect(start).toBeLessThan(firstEnd);
}
});
Comment thread
wenshao marked this conversation as resolved.
Resolved

it('should partition mixed safe/unsafe tools into correct batches', async () => {
const executionLog: string[] = [];

const readTool = new MockTool({
name: 'read_file',
kind: Kind.Read,
execute: async (params) => {
const id = (params as { id: string }).id;
executionLog.push(`read:start:${id}`);
await new Promise((r) => setTimeout(r, 50));
executionLog.push(`read:end:${id}`);
return {
llmContent: `Read ${id} done`,
returnDisplay: `Read ${id} done`,
};
},
});

const editTool = new MockTool({
name: 'edit',
kind: Kind.Edit,
execute: async (params) => {
const id = (params as { id: string }).id;
executionLog.push(`edit:start:${id}`);
await new Promise((r) => setTimeout(r, 20));
executionLog.push(`edit:end:${id}`);
return {
llmContent: `Edit ${id} done`,
returnDisplay: `Edit ${id} done`,
};
},
});

const tools = new Map<string, MockTool>([
['read_file', readTool],
['edit', editTool],
]);
const onAllToolCallsComplete = vi.fn();
const onToolCallsUpdate = vi.fn();
const scheduler = createScheduler(
tools,
onAllToolCallsComplete,
onToolCallsUpdate,
);

// [Read₁, Read₂, Edit, Read₃]
// Expected batches: [Read₁,Read₂](parallel) → [Edit](seq) → [Read₃](seq)
const requests = [
{
callId: '1',
name: 'read_file',
args: { id: '1' },
isClientInitiated: false,
prompt_id: 'p1',
},
{
callId: '2',
name: 'read_file',
args: { id: '2' },
isClientInitiated: false,
prompt_id: 'p1',
},
{
callId: '3',
name: 'edit',
args: { id: 'E' },
isClientInitiated: false,
prompt_id: 'p1',
},
{
callId: '4',
name: 'read_file',
args: { id: '3' },
isClientInitiated: false,
prompt_id: 'p1',
},
];

await scheduler.schedule(requests, new AbortController().signal);

expect(onAllToolCallsComplete).toHaveBeenCalled();
const completedCalls = onAllToolCallsComplete.mock
.calls[0][0] as ToolCall[];
expect(completedCalls).toHaveLength(4);
expect(completedCalls.every((c) => c.status === 'success')).toBe(true);

// Batch 1: Read₁ and Read₂ run in parallel (both start before either ends)
const read1Start = executionLog.indexOf('read:start:1');
const read2Start = executionLog.indexOf('read:start:2');
const firstReadEnd = Math.min(
executionLog.indexOf('read:end:1'),
executionLog.indexOf('read:end:2'),
);
expect(read1Start).toBeLessThan(firstReadEnd);
expect(read2Start).toBeLessThan(firstReadEnd);

// Batch 2: Edit starts after both reads complete
const lastReadEnd = Math.max(
executionLog.indexOf('read:end:1'),
executionLog.indexOf('read:end:2'),
);
const editStart = executionLog.indexOf('edit:start:E');
expect(editStart).toBeGreaterThan(lastReadEnd);

// Batch 3: Read₃ starts after Edit completes
const editEnd = executionLog.indexOf('edit:end:E');
const read3Start = executionLog.indexOf('read:start:3');
expect(read3Start).toBeGreaterThan(editEnd);
});
});
});
115 changes: 95 additions & 20 deletions packages/core/src/core/coreToolScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ import type {
PartListUnion,
} from '@google/genai';
import { ToolNames } from '../tools/tool-names.js';
import { CONCURRENCY_SAFE_KINDS } from '../tools/tools.js';
import { isShellCommandReadOnly } from '../utils/shellReadOnlyChecker.js';
import { stripShellWrapper } from '../utils/shell-utils.js';
import {
buildPermissionCheckContext,
evaluatePermissionRules,
Expand Down Expand Up @@ -329,6 +332,58 @@ interface CoreToolSchedulerOptions {
chatRecordingService?: ChatRecordingService;
}

// ─── Tool Concurrency Helpers ────────────────────────────────

interface ToolBatch {
concurrent: boolean;
calls: ScheduledToolCall[];
}

/**
* Returns true if a scheduled tool call can safely execute concurrently
* with other safe tools (no side effects, no shared mutable state).
*/
function isConcurrencySafe(call: ScheduledToolCall): boolean {
// Agent tools spawn independent sub-agents with no shared state.
if (call.request.name === ToolNames.AGENT) return true;
// Shell commands: check if the command is read-only (e.g., git log, cat).
// Uses the synchronous regex+shell-quote checker (not the async AST-based
// one) because partitioning runs synchronously. The sync checker covers
// the same command whitelist and is fail-closed — unknown commands remain
// sequential. The AST version is used separately for permission decisions.
if (call.tool.kind === Kind.Execute) {
const command = (call.request.args as { command?: string }).command;
if (typeof command !== 'string') return false;
try {
return isShellCommandReadOnly(stripShellWrapper(command));
} catch {
return false; // fail-closed
}
}
Comment thread
wenshao marked this conversation as resolved.
Resolved
Comment thread
wenshao marked this conversation as resolved.
Resolved
return CONCURRENCY_SAFE_KINDS.has(call.tool.kind);
}
Comment thread
wenshao marked this conversation as resolved.
Resolved

/**
* Partition tool calls into consecutive batches by concurrency safety.
*
* Consecutive safe tools are merged into a single parallel batch.
* Each unsafe tool forms its own sequential batch.
*
* Example: [Read, Read, Edit, Read] → [[Read,Read](parallel), [Edit](seq), [Read](seq)]
*/
function partitionToolCalls(calls: ScheduledToolCall[]): ToolBatch[] {
return calls.reduce<ToolBatch[]>((batches, call) => {
const safe = isConcurrencySafe(call);
const lastBatch = batches[batches.length - 1];
if (safe && lastBatch?.concurrent) {
lastBatch.calls.push(call);
} else {
batches.push({ concurrent: safe, calls: [call] });
}
return batches;
}, []);
}

export class CoreToolScheduler {
private toolRegistry: ToolRegistry;
private toolCalls: ToolCall[] = [];
Expand Down Expand Up @@ -1286,32 +1341,52 @@ export class CoreToolScheduler {

if (allCallsFinalOrScheduled) {
const callsToExecute = this.toolCalls.filter(
(call) => call.status === 'scheduled',
(call): call is ScheduledToolCall => call.status === 'scheduled',
);

// Task tools are safe to run concurrently — they spawn independent
// sub-agents with no shared mutable state. All other tools run
// sequentially in their original order to preserve any implicit
// ordering the model may rely on.
const taskCalls = callsToExecute.filter(
(call) => call.request.name === ToolNames.AGENT,
);
const otherCalls = callsToExecute.filter(
(call) => call.request.name !== ToolNames.AGENT,
);

const taskPromise = Promise.all(
taskCalls.map((tc) => this.executeSingleToolCall(tc, signal)),
);
// Partition tool calls into consecutive batches by concurrency safety.
// Consecutive safe tools (Read, Search, Fetch, Agent) are grouped
// into parallel batches; unsafe tools (Edit, Execute, etc.) each form
// their own sequential batch. This preserves ordering semantics while
// allowing read-only tools to execute in parallel.
Comment thread
wenshao marked this conversation as resolved.
Resolved
const batches = partitionToolCalls(callsToExecute);

const othersPromise = (async () => {
for (const toolCall of otherCalls) {
await this.executeSingleToolCall(toolCall, signal);
for (const batch of batches) {
if (batch.concurrent && batch.calls.length > 1) {
await this.runConcurrently(batch.calls, signal);
Comment thread
wenshao marked this conversation as resolved.
Resolved
} else {
for (const call of batch.calls) {
await this.executeSingleToolCall(call, signal);
}
}
})();
}
}
}

await Promise.all([taskPromise, othersPromise]);
/**
* Execute multiple tool calls concurrently with a concurrency cap.
*/
private async runConcurrently(
calls: ScheduledToolCall[],
signal: AbortSignal,
): Promise<void> {
const parsed = parseInt(
process.env['QWEN_CODE_MAX_TOOL_CONCURRENCY'] || '',
10,
);
const maxConcurrency = Number.isFinite(parsed) && parsed >= 1 ? parsed : 10;
const executing = new Set<Promise<void>>();

for (const call of calls) {
const p = this.executeSingleToolCall(call, signal).finally(() => {
executing.delete(p);
});
executing.add(p);
if (executing.size >= maxConcurrency) {
await Promise.race(executing);
}
Comment thread
wenshao marked this conversation as resolved.
Resolved
Comment thread
wenshao marked this conversation as resolved.
Resolved
}
await Promise.all(executing);
}

private async executeSingleToolCall(
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/test-utils/mock-tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface MockToolOptions {
name: string;
displayName?: string;
description?: string;
kind?: Kind;
canUpdateOutput?: boolean;
isOutputMarkdown?: boolean;
getDefaultPermission?: () => Promise<PermissionDecision>;
Expand Down Expand Up @@ -97,7 +98,7 @@ export class MockTool extends BaseDeclarativeTool<
options.name,
options.displayName ?? options.name,
options.description ?? options.name,
Kind.Other,
options.kind ?? Kind.Other,
options.params,
options.isOutputMarkdown ?? false,
options.canUpdateOutput ?? false,
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/tools/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,17 @@ export const MUTATOR_KINDS: Kind[] = [
Kind.Execute,
] as const;

/**
* Tool kinds that are safe to execute concurrently (pure reads, no writes).
* Kind.Think is excluded because some Think tools write to disk
* (e.g., save_memory, todo_write).
*/
export const CONCURRENCY_SAFE_KINDS: ReadonlySet<Kind> = new Set([
Kind.Read,
Kind.Search,
Kind.Fetch,
]);
Comment thread
wenshao marked this conversation as resolved.
Resolved

export interface ToolLocation {
// Absolute path to the file
path: string;
Expand Down
Loading