Skip to content

Commit c405434

Browse files
authored
Fix: TaskTool Dynamic Updates (QwenLM#697)
1 parent 673854b commit c405434

File tree

5 files changed

+106
-24
lines changed

5 files changed

+106
-24
lines changed

packages/core/src/core/client.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ describe('Gemini Client (client.ts)', () => {
239239
};
240240
const mockSubagentManager = {
241241
listSubagents: vi.fn().mockResolvedValue([]),
242+
addChangeListener: vi.fn().mockReturnValue(() => {}),
242243
};
243244
mockConfigObject = {
244245
getContentGeneratorConfig: vi

packages/core/src/subagents/subagent-manager.ts

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,29 @@ const AGENT_CONFIG_DIR = 'agents';
4040
export class SubagentManager {
4141
private readonly validator: SubagentValidator;
4242
private subagentsCache: Map<SubagentLevel, SubagentConfig[]> | null = null;
43+
private readonly changeListeners: Set<() => void> = new Set();
4344

4445
constructor(private readonly config: Config) {
4546
this.validator = new SubagentValidator();
4647
}
4748

49+
addChangeListener(listener: () => void): () => void {
50+
this.changeListeners.add(listener);
51+
return () => {
52+
this.changeListeners.delete(listener);
53+
};
54+
}
55+
56+
private notifyChangeListeners(): void {
57+
for (const listener of this.changeListeners) {
58+
try {
59+
listener();
60+
} catch (error) {
61+
console.warn('Subagent change listener threw an error:', error);
62+
}
63+
}
64+
}
65+
4866
/**
4967
* Creates a new subagent configuration.
5068
*
@@ -93,8 +111,8 @@ export class SubagentManager {
93111

94112
try {
95113
await fs.writeFile(filePath, content, 'utf8');
96-
// Clear cache after successful creation
97-
this.clearCache();
114+
// Refresh cache after successful creation
115+
await this.refreshCache();
98116
} catch (error) {
99117
throw new SubagentError(
100118
`Failed to write subagent file: ${error instanceof Error ? error.message : 'Unknown error'}`,
@@ -183,8 +201,8 @@ export class SubagentManager {
183201

184202
try {
185203
await fs.writeFile(existing.filePath, content, 'utf8');
186-
// Clear cache after successful update
187-
this.clearCache();
204+
// Refresh cache after successful update
205+
await this.refreshCache();
188206
} catch (error) {
189207
throw new SubagentError(
190208
`Failed to update subagent file: ${error instanceof Error ? error.message : 'Unknown error'}`,
@@ -242,8 +260,8 @@ export class SubagentManager {
242260
);
243261
}
244262

245-
// Clear cache after successful deletion
246-
this.clearCache();
263+
// Refresh cache after successful deletion
264+
await this.refreshCache();
247265
}
248266

249267
/**
@@ -327,21 +345,17 @@ export class SubagentManager {
327345
* @private
328346
*/
329347
private async refreshCache(): Promise<void> {
330-
this.subagentsCache = new Map();
348+
const subagentsCache = new Map();
331349

332350
const levels: SubagentLevel[] = ['project', 'user', 'builtin'];
333351

334352
for (const level of levels) {
335353
const levelSubagents = await this.listSubagentsAtLevel(level);
336-
this.subagentsCache.set(level, levelSubagents);
354+
subagentsCache.set(level, levelSubagents);
337355
}
338-
}
339356

340-
/**
341-
* Clears the subagents cache, forcing the next listSubagents call to reload from disk.
342-
*/
343-
clearCache(): void {
344-
this.subagentsCache = null;
357+
this.subagentsCache = subagentsCache;
358+
this.notifyChangeListeners();
345359
}
346360

347361
/**

packages/core/src/subagents/subagent.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,14 @@ import type {
4141
ToolConfig,
4242
} from './types.js';
4343
import { SubagentTerminateMode } from './types.js';
44+
import { GeminiClient } from '../core/client.js';
4445

4546
vi.mock('../core/geminiChat.js');
4647
vi.mock('../core/contentGenerator.js');
4748
vi.mock('../utils/environmentContext.js');
4849
vi.mock('../core/nonInteractiveToolExecutor.js');
4950
vi.mock('../ide/ide-client.js');
51+
vi.mock('../core/client.js');
5052

5153
async function createMockConfig(
5254
toolRegistryMocks = {},
@@ -194,6 +196,28 @@ describe('subagent.ts', () => {
194196
}) as unknown as GeminiChat,
195197
);
196198

199+
// Mock GeminiClient constructor to return a properly mocked client
200+
const mockGeminiChat = {
201+
setTools: vi.fn(),
202+
getHistory: vi.fn().mockReturnValue([]),
203+
setHistory: vi.fn(),
204+
sendMessageStream: vi.fn(),
205+
};
206+
207+
const mockGeminiClient = {
208+
getChat: vi.fn().mockReturnValue(mockGeminiChat),
209+
setTools: vi.fn().mockResolvedValue(undefined),
210+
isInitialized: vi.fn().mockReturnValue(true),
211+
getHistory: vi.fn().mockReturnValue([]),
212+
initialize: vi.fn().mockResolvedValue(undefined),
213+
setHistory: vi.fn(),
214+
};
215+
216+
// Mock the GeminiClient constructor
217+
vi.mocked(GeminiClient).mockImplementation(
218+
() => mockGeminiClient as unknown as GeminiClient,
219+
);
220+
197221
// Default mock for executeToolCall
198222
vi.mocked(executeToolCall).mockResolvedValue({
199223
callId: 'default-call',

packages/core/src/tools/task.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ describe('TaskTool', () => {
4343
let config: Config;
4444
let taskTool: TaskTool;
4545
let mockSubagentManager: SubagentManager;
46+
let changeListeners: Array<() => void>;
4647

4748
const mockSubagents: SubagentConfig[] = [
4849
{
@@ -70,13 +71,25 @@ describe('TaskTool', () => {
7071
getProjectRoot: vi.fn().mockReturnValue('/test/project'),
7172
getSessionId: vi.fn().mockReturnValue('test-session-id'),
7273
getSubagentManager: vi.fn(),
74+
getGeminiClient: vi.fn().mockReturnValue(undefined),
7375
} as unknown as Config;
7476

77+
changeListeners = [];
78+
7579
// Setup SubagentManager mock
7680
mockSubagentManager = {
7781
listSubagents: vi.fn().mockResolvedValue(mockSubagents),
7882
loadSubagent: vi.fn(),
7983
createSubagentScope: vi.fn(),
84+
addChangeListener: vi.fn((listener: () => void) => {
85+
changeListeners.push(listener);
86+
return () => {
87+
const index = changeListeners.indexOf(listener);
88+
if (index >= 0) {
89+
changeListeners.splice(index, 1);
90+
}
91+
};
92+
}),
8093
} as unknown as SubagentManager;
8194

8295
MockedSubagentManager.mockImplementation(() => mockSubagentManager);
@@ -106,6 +119,10 @@ describe('TaskTool', () => {
106119
expect(mockSubagentManager.listSubagents).toHaveBeenCalled();
107120
});
108121

122+
it('should subscribe to subagent manager changes', () => {
123+
expect(mockSubagentManager.addChangeListener).toHaveBeenCalledTimes(1);
124+
});
125+
109126
it('should update description with available subagents', () => {
110127
expect(taskTool.description).toContain('file-search');
111128
expect(taskTool.description).toContain(
@@ -232,6 +249,31 @@ describe('TaskTool', () => {
232249
});
233250

234251
describe('refreshSubagents', () => {
252+
it('should refresh when change listener fires', async () => {
253+
const newSubagents: SubagentConfig[] = [
254+
{
255+
name: 'new-agent',
256+
description: 'A brand new agent',
257+
systemPrompt: 'Do new things.',
258+
level: 'project',
259+
filePath: '/project/.qwen/agents/new-agent.md',
260+
},
261+
];
262+
263+
vi.mocked(mockSubagentManager.listSubagents).mockResolvedValueOnce(
264+
newSubagents,
265+
);
266+
267+
const listener = changeListeners[0];
268+
expect(listener).toBeDefined();
269+
270+
listener?.();
271+
await vi.runAllTimersAsync();
272+
273+
expect(taskTool.description).toContain('new-agent');
274+
expect(taskTool.description).toContain('A brand new agent');
275+
});
276+
235277
it('should refresh available subagents and update description', async () => {
236278
const newSubagents: SubagentConfig[] = [
237279
{

packages/core/src/tools/task.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,23 +86,32 @@ export class TaskTool extends BaseDeclarativeTool<TaskParams, ToolResult> {
8686
);
8787

8888
this.subagentManager = config.getSubagentManager();
89+
this.subagentManager.addChangeListener(() => {
90+
void this.refreshSubagents();
91+
});
8992

9093
// Initialize the tool asynchronously
91-
this.initializeAsync();
94+
this.refreshSubagents();
9295
}
9396

9497
/**
9598
* Asynchronously initializes the tool by loading available subagents
9699
* and updating the description and schema.
97100
*/
98-
private async initializeAsync(): Promise<void> {
101+
async refreshSubagents(): Promise<void> {
99102
try {
100103
this.availableSubagents = await this.subagentManager.listSubagents();
101104
this.updateDescriptionAndSchema();
102105
} catch (error) {
103106
console.warn('Failed to load subagents for Task tool:', error);
104107
this.availableSubagents = [];
105108
this.updateDescriptionAndSchema();
109+
} finally {
110+
// Update the client with the new tools
111+
const geminiClient = this.config.getGeminiClient();
112+
if (geminiClient) {
113+
await geminiClient.setTools();
114+
}
106115
}
107116
}
108117

@@ -201,14 +210,6 @@ assistant: "I'm going to use the Task tool to launch the with the greeting-respo
201210
}
202211
}
203212

204-
/**
205-
* Refreshes the available subagents and updates the tool description.
206-
* This can be called when subagents are added or removed.
207-
*/
208-
async refreshSubagents(): Promise<void> {
209-
await this.initializeAsync();
210-
}
211-
212213
override validateToolParams(params: TaskParams): string | null {
213214
// Validate required fields
214215
if (

0 commit comments

Comments
 (0)