Skip to content

Commit eda4640

Browse files
committed
test: fix backgroundTools.cleanup test for AgentTracker integration
Update the test to mock the new agentTracker module instead of agentStates. This ensures the test correctly verifies that the backgroundTools.cleanupSubAgent method properly delegates to agentTracker.terminateAgent.
1 parent 9defbad commit eda4640

File tree

6 files changed

+353
-94
lines changed

6 files changed

+353
-94
lines changed

packages/agent/src/core/backgroundTools.cleanup.test.ts

Lines changed: 57 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,36 +2,27 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
22

33
// Import mocked modules
44
import { BrowserManager } from '../tools/browser/BrowserManager.js';
5-
import { agentStates } from '../tools/interaction/agentStart.js';
5+
import { agentTracker } from '../tools/interaction/agentTracker.js';
66
import { processStates } from '../tools/system/shellStart.js';
77

88
import { BackgroundTools, BackgroundToolStatus } from './backgroundTools';
9-
import { Tool } from './types';
9+
10+
// Import the ChildProcess type for mocking
11+
import type { ChildProcess } from 'child_process';
1012

1113
// Define types for our mocks that match the actual types
1214
type MockProcessState = {
13-
process: { kill: ReturnType<typeof vi.fn> };
14-
state: { completed: boolean };
15-
command?: string;
16-
stdout?: string[];
17-
stderr?: string[];
18-
showStdIn?: boolean;
19-
showStdout?: boolean;
20-
};
21-
22-
type MockAgentState = {
23-
aborted: boolean;
24-
completed: boolean;
25-
context: {
26-
backgroundTools: {
27-
cleanup: ReturnType<typeof vi.fn>;
28-
};
15+
process: ChildProcess & { kill: ReturnType<typeof vi.fn> };
16+
state: {
17+
completed: boolean;
18+
signaled: boolean;
19+
exitCode: number | null;
2920
};
30-
goal?: string;
31-
prompt?: string;
32-
output?: string;
33-
workingDirectory?: string;
34-
tools?: Tool[];
21+
command: string;
22+
stdout: string[];
23+
stderr: string[];
24+
showStdIn: boolean;
25+
showStdout: boolean;
3526
};
3627

3728
// Mock dependencies
@@ -49,9 +40,28 @@ vi.mock('../tools/system/shellStart.js', () => {
4940
};
5041
});
5142

52-
vi.mock('../tools/interaction/agentStart.js', () => {
43+
vi.mock('../tools/interaction/agentTracker.js', () => {
5344
return {
54-
agentStates: new Map<string, MockAgentState>(),
45+
agentTracker: {
46+
terminateAgent: vi.fn().mockResolvedValue(undefined),
47+
getAgentState: vi.fn().mockImplementation((id: string) => {
48+
return {
49+
id,
50+
aborted: false,
51+
completed: false,
52+
context: {
53+
backgroundTools: {
54+
cleanup: vi.fn().mockResolvedValue(undefined),
55+
},
56+
},
57+
goal: 'test goal',
58+
prompt: 'test prompt',
59+
output: '',
60+
workingDirectory: '/test',
61+
tools: [],
62+
};
63+
}),
64+
},
5565
};
5666
});
5767

@@ -75,11 +85,19 @@ describe('BackgroundTools cleanup', () => {
7585
// Setup mock process states
7686
const mockProcess = {
7787
kill: vi.fn(),
78-
};
88+
stdin: null,
89+
stdout: null,
90+
stderr: null,
91+
stdio: null,
92+
} as unknown as ChildProcess & { kill: ReturnType<typeof vi.fn> };
7993

8094
const mockProcessState: MockProcessState = {
8195
process: mockProcess,
82-
state: { completed: false },
96+
state: {
97+
completed: false,
98+
signaled: false,
99+
exitCode: null,
100+
},
83101
command: 'test command',
84102
stdout: [],
85103
stderr: [],
@@ -88,26 +106,13 @@ describe('BackgroundTools cleanup', () => {
88106
};
89107

90108
processStates.clear();
91-
processStates.set('shell-1', mockProcessState as any);
92-
93-
// Setup mock agent states
94-
const mockAgentState: MockAgentState = {
95-
aborted: false,
96-
completed: false,
97-
context: {
98-
backgroundTools: {
99-
cleanup: vi.fn().mockResolvedValue(undefined),
100-
},
101-
},
102-
goal: 'test goal',
103-
prompt: 'test prompt',
104-
output: '',
105-
workingDirectory: '/test',
106-
tools: [],
107-
};
109+
processStates.set(
110+
'shell-1',
111+
mockProcessState as unknown as MockProcessState,
112+
);
108113

109-
agentStates.clear();
110-
agentStates.set('agent-1', mockAgentState as any);
114+
// Reset the agentTracker mock
115+
vi.mocked(agentTracker.terminateAgent).mockClear();
111116
});
112117

113118
afterEach(() => {
@@ -120,7 +125,6 @@ describe('BackgroundTools cleanup', () => {
120125

121126
// Clear mock states
122127
processStates.clear();
123-
agentStates.clear();
124128
});
125129

126130
it('should clean up browser sessions', async () => {
@@ -149,7 +153,10 @@ describe('BackgroundTools cleanup', () => {
149153
const mockProcessState = processStates.get('shell-1');
150154

151155
// Set the shell ID to match
152-
processStates.set(shellId, processStates.get('shell-1') as any);
156+
processStates.set(
157+
shellId,
158+
processStates.get('shell-1') as unknown as MockProcessState,
159+
);
153160

154161
// Run cleanup
155162
await backgroundTools.cleanup();
@@ -166,21 +173,11 @@ describe('BackgroundTools cleanup', () => {
166173
// Register an agent tool
167174
const agentId = backgroundTools.registerAgent('Test goal');
168175

169-
// Get mock agent state
170-
const mockAgentState = agentStates.get('agent-1');
171-
172-
// Set the agent ID to match
173-
agentStates.set(agentId, agentStates.get('agent-1') as any);
174-
175176
// Run cleanup
176177
await backgroundTools.cleanup();
177178

178-
// Check that agent was marked as aborted
179-
expect(mockAgentState?.aborted).toBe(true);
180-
expect(mockAgentState?.completed).toBe(true);
181-
182-
// Check that cleanup was called on the agent's background tools
183-
expect(mockAgentState?.context.backgroundTools.cleanup).toHaveBeenCalled();
179+
// Check that terminateAgent was called with the agent ID
180+
expect(agentTracker.terminateAgent).toHaveBeenCalledWith(agentId);
184181

185182
// Check that tool status was updated
186183
const tool = backgroundTools.getToolById(agentId);

packages/agent/src/core/backgroundTools.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { v4 as uuidv4 } from 'uuid';
22

33
// These imports will be used by the cleanup method
44
import { BrowserManager } from '../tools/browser/BrowserManager.js';
5-
import { agentStates } from '../tools/interaction/agentStart.js';
5+
import { agentTracker } from '../tools/interaction/agentTracker.js';
66
import { processStates } from '../tools/system/shellStart.js';
77

88
// Types of background processes we can track
@@ -268,15 +268,8 @@ export class BackgroundTools {
268268
*/
269269
private async cleanupSubAgent(tool: AgentBackgroundTool): Promise<void> {
270270
try {
271-
const agentState = agentStates.get(tool.id);
272-
if (agentState && !agentState.aborted) {
273-
// Set the agent as aborted and completed
274-
agentState.aborted = true;
275-
agentState.completed = true;
276-
277-
// Clean up resources owned by this sub-agent
278-
await agentState.context.backgroundTools.cleanup();
279-
}
271+
// Delegate to the agent tracker
272+
await agentTracker.terminateAgent(tool.id);
280273
this.updateToolStatus(tool.id, BackgroundToolStatus.TERMINATED);
281274
} catch (error) {
282275
this.updateToolStatus(tool.id, BackgroundToolStatus.ERROR, {

packages/agent/src/tools/getTools.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { userPromptTool } from './interaction/userPrompt.js';
99
import { fetchTool } from './io/fetch.js';
1010
import { textEditorTool } from './io/textEditor.js';
1111
import { createMcpTool } from './mcp.js';
12+
import { listAgentsTool } from './system/listAgents.js';
1213
import { listBackgroundToolsTool } from './system/listBackgroundTools.js';
1314
import { sequenceCompleteTool } from './system/sequenceComplete.js';
1415
import { shellMessageTool } from './system/shellMessage.js';
@@ -41,6 +42,7 @@ export function getTools(options?: GetToolsOptions): Tool[] {
4142
//respawnTool as unknown as Tool, this is a confusing tool for now.
4243
sleepTool as unknown as Tool,
4344
listBackgroundToolsTool as unknown as Tool,
45+
listAgentsTool as unknown as Tool,
4446
];
4547

4648
// Only include userPrompt tool if enabled

packages/agent/src/tools/interaction/agentStart.ts

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { v4 as uuidv4 } from 'uuid';
21
import { z } from 'zod';
32
import { zodToJsonSchema } from 'zod-to-json-schema';
43

@@ -8,26 +7,13 @@ import {
87
AgentConfig,
98
} from '../../core/toolAgent/config.js';
109
import { toolAgent } from '../../core/toolAgent/toolAgentCore.js';
11-
import { ToolAgentResult } from '../../core/toolAgent/types.js';
1210
import { Tool, ToolContext } from '../../core/types.js';
1311
import { getTools } from '../getTools.js';
1412

15-
// Define AgentState type
16-
type AgentState = {
17-
goal: string;
18-
prompt: string;
19-
output: string;
20-
completed: boolean;
21-
error?: string;
22-
result?: ToolAgentResult;
23-
context: ToolContext;
24-
workingDirectory: string;
25-
tools: Tool[];
26-
aborted: boolean;
27-
};
13+
import { AgentStatus, agentTracker, AgentState } from './agentTracker.js';
2814

29-
// Global map to store agent state
30-
export const agentStates: Map<string, AgentState> = new Map();
15+
// For backward compatibility
16+
export const agentStates = new Map<string, AgentState>();
3117

3218
const parameterSchema = z.object({
3319
description: z
@@ -100,11 +86,12 @@ export const agentStartTool: Tool<Parameters, ReturnType> = {
10086
userPrompt = false,
10187
} = parameterSchema.parse(params);
10288

103-
// Create an instance ID
104-
const instanceId = uuidv4();
89+
// Register this agent with the agent tracker
90+
const instanceId = agentTracker.registerAgent(goal);
10591

106-
// Register this agent with the background tool registry
92+
// For backward compatibility, also register with background tools
10793
backgroundTools.registerAgent(goal);
94+
10895
logger.verbose(`Registered agent with ID: ${instanceId}`);
10996

11097
// Construct a well-structured prompt
@@ -124,6 +111,7 @@ export const agentStartTool: Tool<Parameters, ReturnType> = {
124111

125112
// Store the agent state
126113
const agentState: AgentState = {
114+
id: instanceId,
127115
goal,
128116
prompt,
129117
output: '',
@@ -134,6 +122,10 @@ export const agentStartTool: Tool<Parameters, ReturnType> = {
134122
aborted: false,
135123
};
136124

125+
// Register agent state with the tracker
126+
agentTracker.registerAgentState(instanceId, agentState);
127+
128+
// For backward compatibility
137129
agentStates.set(instanceId, agentState);
138130

139131
// Start the agent in a separate promise that we don't await
@@ -146,13 +138,20 @@ export const agentStartTool: Tool<Parameters, ReturnType> = {
146138
});
147139

148140
// Update agent state with the result
149-
const state = agentStates.get(instanceId);
141+
const state = agentTracker.getAgentState(instanceId);
150142
if (state && !state.aborted) {
151143
state.completed = true;
152144
state.result = result;
153145
state.output = result.result;
154146

155-
// Update background tool registry with completed status
147+
// Update agent tracker with completed status
148+
agentTracker.updateAgentStatus(instanceId, AgentStatus.COMPLETED, {
149+
result:
150+
result.result.substring(0, 100) +
151+
(result.result.length > 100 ? '...' : ''),
152+
});
153+
154+
// For backward compatibility
156155
backgroundTools.updateToolStatus(
157156
instanceId,
158157
BackgroundToolStatus.COMPLETED,
@@ -168,12 +167,17 @@ export const agentStartTool: Tool<Parameters, ReturnType> = {
168167
}
169168
} catch (error) {
170169
// Update agent state with the error
171-
const state = agentStates.get(instanceId);
170+
const state = agentTracker.getAgentState(instanceId);
172171
if (state && !state.aborted) {
173172
state.completed = true;
174173
state.error = error instanceof Error ? error.message : String(error);
175174

176-
// Update background tool registry with error status
175+
// Update agent tracker with error status
176+
agentTracker.updateAgentStatus(instanceId, AgentStatus.ERROR, {
177+
error: error instanceof Error ? error.message : String(error),
178+
});
179+
180+
// For backward compatibility
177181
backgroundTools.updateToolStatus(
178182
instanceId,
179183
BackgroundToolStatus.ERROR,

0 commit comments

Comments
 (0)