Skip to content

Commit 6351352

Browse files
authored
feat(core): Implement parallel FC for read only tools. (google-gemini#18791)
1 parent 2ac39b6 commit 6351352

11 files changed

Lines changed: 862 additions & 301 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"read_file","args":{"file_path":"file1.txt"}}},{"functionCall":{"name":"read_file","args":{"file_path":"file2.txt"}}},{"functionCall":{"name":"write_file","args":{"file_path":"output.txt","content":"wave2"}}},{"functionCall":{"name":"read_file","args":{"file_path":"file3.txt"}}},{"functionCall":{"name":"read_file","args":{"file_path":"file4.txt"}}}, {"text":"All waves completed successfully."}]},"finishReason":"STOP","index":0}]}]}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* @license
3+
* Copyright 2026 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
8+
import { TestRig } from './test-helper.js';
9+
import { join } from 'node:path';
10+
import fs from 'node:fs';
11+
12+
describe('Parallel Tool Execution Integration', () => {
13+
let rig: TestRig;
14+
15+
beforeEach(() => {
16+
rig = new TestRig();
17+
});
18+
19+
afterEach(async () => {
20+
await rig.cleanup();
21+
});
22+
23+
it('should execute [read, read, write, read, read] in correct waves with user approval', async () => {
24+
rig.setup('parallel-wave-execution', {
25+
fakeResponsesPath: join(import.meta.dirname, 'parallel-tools.responses'),
26+
settings: {
27+
tools: {
28+
core: ['read_file', 'write_file'],
29+
approval: 'ASK', // Disable YOLO mode to show permission prompts
30+
confirmationRequired: ['write_file'],
31+
},
32+
},
33+
});
34+
35+
rig.createFile('file1.txt', 'c1');
36+
rig.createFile('file2.txt', 'c2');
37+
rig.createFile('file3.txt', 'c3');
38+
rig.createFile('file4.txt', 'c4');
39+
rig.sync();
40+
41+
const run = await rig.runInteractive({ approvalMode: 'default' });
42+
43+
// 1. Trigger the wave
44+
await run.type('ok');
45+
await run.type('\r');
46+
47+
// 3. Wait for the write_file prompt.
48+
await run.expectText('Allow', 5000);
49+
50+
// 4. Press Enter to approve the write_file.
51+
await run.type('y');
52+
await run.type('\r');
53+
54+
// 5. Wait for the final model response
55+
await run.expectText('All waves completed successfully.', 5000);
56+
57+
// Verify all tool calls were made and succeeded in the logs
58+
await rig.expectToolCallSuccess(['write_file']);
59+
const toolLogs = rig.readToolLogs();
60+
61+
const readFiles = toolLogs.filter(
62+
(l) => l.toolRequest.name === 'read_file',
63+
);
64+
const writeFiles = toolLogs.filter(
65+
(l) => l.toolRequest.name === 'write_file',
66+
);
67+
68+
expect(readFiles.length).toBe(4);
69+
expect(writeFiles.length).toBe(1);
70+
expect(toolLogs.every((l) => l.toolRequest.success)).toBe(true);
71+
72+
// Check that output.txt was actually written
73+
expect(fs.readFileSync(join(rig.testDir!, 'output.txt'), 'utf8')).toBe(
74+
'wave2',
75+
);
76+
});
77+
});

packages/core/src/agents/subagent-tool.test.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ import { createMockMessageBus } from '../test-utils/mock-message-bus.js';
1717
import type { Config } from '../config/config.js';
1818
import type { MessageBus } from '../confirmation-bus/message-bus.js';
1919
import type {
20+
DeclarativeTool,
2021
ToolCallConfirmationDetails,
2122
ToolInvocation,
2223
ToolResult,
2324
} from '../tools/tools.js';
25+
import type { ToolRegistry } from 'src/tools/tool-registry.js';
2426

2527
vi.mock('./subagent-tool-wrapper.js');
2628

@@ -274,3 +276,85 @@ describe('SubAgentInvocation', () => {
274276
});
275277
});
276278
});
279+
280+
describe('SubagentTool Read-Only logic', () => {
281+
let mockConfig: Config;
282+
let mockMessageBus: MessageBus;
283+
284+
beforeEach(() => {
285+
vi.clearAllMocks();
286+
mockConfig = makeFakeConfig();
287+
mockMessageBus = createMockMessageBus();
288+
});
289+
290+
it('should be false for remote agents', () => {
291+
const tool = new SubagentTool(
292+
testRemoteDefinition,
293+
mockConfig,
294+
mockMessageBus,
295+
);
296+
expect(tool.isReadOnly).toBe(false);
297+
});
298+
299+
it('should be true for local agent with only read-only tools', () => {
300+
const readOnlyTool = {
301+
name: 'read',
302+
isReadOnly: true,
303+
} as unknown as DeclarativeTool<object, ToolResult>;
304+
const registry = {
305+
getTool: (name: string) => (name === 'read' ? readOnlyTool : undefined),
306+
};
307+
vi.spyOn(mockConfig, 'getToolRegistry').mockReturnValue(
308+
registry as unknown as ToolRegistry,
309+
);
310+
311+
const defWithTools: LocalAgentDefinition = {
312+
...testDefinition,
313+
toolConfig: { tools: ['read'] },
314+
};
315+
const tool = new SubagentTool(defWithTools, mockConfig, mockMessageBus);
316+
expect(tool.isReadOnly).toBe(true);
317+
});
318+
319+
it('should be false for local agent with at least one non-read-only tool', () => {
320+
const readOnlyTool = {
321+
name: 'read',
322+
isReadOnly: true,
323+
} as unknown as DeclarativeTool<object, ToolResult>;
324+
const mutatorTool = {
325+
name: 'write',
326+
isReadOnly: false,
327+
} as unknown as DeclarativeTool<object, ToolResult>;
328+
const registry = {
329+
getTool: (name: string) => {
330+
if (name === 'read') return readOnlyTool;
331+
if (name === 'write') return mutatorTool;
332+
return undefined;
333+
},
334+
};
335+
vi.spyOn(mockConfig, 'getToolRegistry').mockReturnValue(
336+
registry as unknown as ToolRegistry,
337+
);
338+
339+
const defWithTools: LocalAgentDefinition = {
340+
...testDefinition,
341+
toolConfig: { tools: ['read', 'write'] },
342+
};
343+
const tool = new SubagentTool(defWithTools, mockConfig, mockMessageBus);
344+
expect(tool.isReadOnly).toBe(false);
345+
});
346+
347+
it('should be true for local agent with no tools', () => {
348+
const registry = { getTool: () => undefined };
349+
vi.spyOn(mockConfig, 'getToolRegistry').mockReturnValue(
350+
registry as unknown as ToolRegistry,
351+
);
352+
353+
const defNoTools: LocalAgentDefinition = {
354+
...testDefinition,
355+
toolConfig: { tools: [] },
356+
};
357+
const tool = new SubagentTool(defNoTools, mockConfig, mockMessageBus);
358+
expect(tool.isReadOnly).toBe(true);
359+
});
360+
});

packages/core/src/agents/subagent-tool.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
type ToolResult,
1212
BaseToolInvocation,
1313
type ToolCallConfirmationDetails,
14+
isTool,
1415
} from '../tools/tools.js';
1516
import type { AnsiOutput } from '../utils/terminalSerializer.js';
1617
import type { Config } from '../config/config.js';
@@ -48,6 +49,53 @@ export class SubagentTool extends BaseDeclarativeTool<AgentInputs, ToolResult> {
4849
);
4950
}
5051

52+
private _memoizedIsReadOnly: boolean | undefined;
53+
54+
override get isReadOnly(): boolean {
55+
if (this._memoizedIsReadOnly !== undefined) {
56+
return this._memoizedIsReadOnly;
57+
}
58+
// No try-catch here. If getToolRegistry() throws, we let it throw.
59+
// This is an invariant: you can't check read-only status if the system isn't initialized.
60+
this._memoizedIsReadOnly = SubagentTool.checkIsReadOnly(
61+
this.definition,
62+
this.config,
63+
);
64+
return this._memoizedIsReadOnly;
65+
}
66+
67+
private static checkIsReadOnly(
68+
definition: AgentDefinition,
69+
config: Config,
70+
): boolean {
71+
if (definition.kind === 'remote') {
72+
return false;
73+
}
74+
const tools = definition.toolConfig?.tools ?? [];
75+
const registry = config.getToolRegistry();
76+
77+
if (!registry) {
78+
return false;
79+
}
80+
81+
for (const tool of tools) {
82+
if (typeof tool === 'string') {
83+
const resolvedTool = registry.getTool(tool);
84+
if (!resolvedTool || !resolvedTool.isReadOnly) {
85+
return false;
86+
}
87+
} else if (isTool(tool)) {
88+
if (!tool.isReadOnly) {
89+
return false;
90+
}
91+
} else {
92+
// FunctionDeclaration - we don't know, so assume NOT read-only
93+
return false;
94+
}
95+
}
96+
return true;
97+
}
98+
5199
protected createInvocation(
52100
params: AgentInputs,
53101
messageBus: MessageBus,

0 commit comments

Comments
 (0)