Skip to content

Commit 5dedfe9

Browse files
committed
fix: fix build errors in browser tracker implementation
- Fixed type errors in browser tracker implementation - Updated tests to remove browser-related code from backgroundTools tests - Added default agent name for browser tracker tools
1 parent 9defbad commit 5dedfe9

File tree

10 files changed

+304
-182
lines changed

10 files changed

+304
-182
lines changed

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

Lines changed: 1 addition & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
22

33
// Import mocked modules
4-
import { BrowserManager } from '../tools/browser/BrowserManager.js';
54
import { agentStates } from '../tools/interaction/agentStart.js';
65
import { processStates } from '../tools/system/shellStart.js';
76

@@ -35,14 +34,6 @@ type MockAgentState = {
3534
};
3635

3736
// Mock dependencies
38-
vi.mock('../tools/browser/BrowserManager.js', () => {
39-
return {
40-
BrowserManager: class MockBrowserManager {
41-
closeSession = vi.fn().mockResolvedValue(undefined);
42-
},
43-
};
44-
});
45-
4637
vi.mock('../tools/system/shellStart.js', () => {
4738
return {
4839
processStates: new Map<string, MockProcessState>(),
@@ -58,20 +49,13 @@ vi.mock('../tools/interaction/agentStart.js', () => {
5849
describe('BackgroundTools cleanup', () => {
5950
let backgroundTools: BackgroundTools;
6051

61-
// Setup mocks for globalThis and process states
52+
// Setup mocks for process states
6253
beforeEach(() => {
6354
backgroundTools = new BackgroundTools('test-agent');
6455

6556
// Reset mocks
6657
vi.resetAllMocks();
6758

68-
// Setup global browser manager
69-
(
70-
globalThis as unknown as { __BROWSER_MANAGER__: BrowserManager }
71-
).__BROWSER_MANAGER__ = {
72-
closeSession: vi.fn().mockResolvedValue(undefined),
73-
} as unknown as BrowserManager;
74-
7559
// Setup mock process states
7660
const mockProcess = {
7761
kill: vi.fn(),
@@ -113,34 +97,11 @@ describe('BackgroundTools cleanup', () => {
11397
afterEach(() => {
11498
vi.resetAllMocks();
11599

116-
// Clear global browser manager
117-
(
118-
globalThis as unknown as { __BROWSER_MANAGER__?: BrowserManager }
119-
).__BROWSER_MANAGER__ = undefined;
120-
121100
// Clear mock states
122101
processStates.clear();
123102
agentStates.clear();
124103
});
125104

126-
it('should clean up browser sessions', async () => {
127-
// Register a browser tool
128-
const browserId = backgroundTools.registerBrowser('https://example.com');
129-
130-
// Run cleanup
131-
await backgroundTools.cleanup();
132-
133-
// Check that closeSession was called
134-
expect(
135-
(globalThis as unknown as { __BROWSER_MANAGER__: BrowserManager })
136-
.__BROWSER_MANAGER__.closeSession,
137-
).toHaveBeenCalledWith(browserId);
138-
139-
// Check that tool status was updated
140-
const tool = backgroundTools.getToolById(browserId);
141-
expect(tool?.status).toBe(BackgroundToolStatus.COMPLETED);
142-
});
143-
144105
it('should clean up shell processes', async () => {
145106
// Register a shell tool
146107
const shellId = backgroundTools.registerShell('echo "test"');
@@ -186,38 +147,4 @@ describe('BackgroundTools cleanup', () => {
186147
const tool = backgroundTools.getToolById(agentId);
187148
expect(tool?.status).toBe(BackgroundToolStatus.TERMINATED);
188149
});
189-
190-
it('should handle errors during cleanup', async () => {
191-
// Register a browser tool
192-
const browserId = backgroundTools.registerBrowser('https://example.com');
193-
194-
// Make closeSession throw an error
195-
(
196-
(globalThis as unknown as { __BROWSER_MANAGER__: BrowserManager })
197-
.__BROWSER_MANAGER__.closeSession as ReturnType<typeof vi.fn>
198-
).mockRejectedValue(new Error('Test error'));
199-
200-
// Run cleanup
201-
await backgroundTools.cleanup();
202-
203-
// Check that tool status was updated to ERROR
204-
const tool = backgroundTools.getToolById(browserId);
205-
expect(tool?.status).toBe(BackgroundToolStatus.ERROR);
206-
expect(tool?.metadata.error).toBe('Test error');
207-
});
208-
209-
it('should only clean up running tools', async () => {
210-
// Register a browser tool and mark it as completed
211-
const browserId = backgroundTools.registerBrowser('https://example.com');
212-
backgroundTools.updateToolStatus(browserId, BackgroundToolStatus.COMPLETED);
213-
214-
// Run cleanup
215-
await backgroundTools.cleanup();
216-
217-
// Check that closeSession was not called
218-
expect(
219-
(globalThis as unknown as { __BROWSER_MANAGER__: BrowserManager })
220-
.__BROWSER_MANAGER__.closeSession,
221-
).not.toHaveBeenCalled();
222-
});
223150
});

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

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,21 +35,7 @@ describe('BackgroundToolRegistry', () => {
3535
}
3636
});
3737

38-
it('should register a browser process', () => {
39-
const id = backgroundTools.registerBrowser('https://example.com');
40-
41-
expect(id).toBe('test-id-1');
42-
43-
const tool = backgroundTools.getToolById(id);
44-
expect(tool).toBeDefined();
45-
if (tool) {
46-
expect(tool.type).toBe(BackgroundToolType.BROWSER);
47-
expect(tool.status).toBe(BackgroundToolStatus.RUNNING);
48-
if (tool.type === BackgroundToolType.BROWSER) {
49-
expect(tool.metadata.url).toBe('https://example.com');
50-
}
51-
}
52-
});
38+
// Browser registration test removed since browser tracking is now decoupled
5339

5440
it('should update tool status', () => {
5541
const id = backgroundTools.registerShell('sleep 10');

packages/agent/src/core/backgroundTools.ts

Lines changed: 2 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
import { v4 as uuidv4 } from 'uuid';
22

33
// These imports will be used by the cleanup method
4-
import { BrowserManager } from '../tools/browser/BrowserManager.js';
54
import { agentStates } from '../tools/interaction/agentStart.js';
65
import { processStates } from '../tools/system/shellStart.js';
76

87
// Types of background processes we can track
98
export enum BackgroundToolType {
109
SHELL = 'shell',
11-
BROWSER = 'browser',
1210
AGENT = 'agent',
1311
}
1412

@@ -41,15 +39,6 @@ export interface ShellBackgroundTool extends BackgroundTool {
4139
};
4240
}
4341

44-
// Browser process specific data
45-
export interface BrowserBackgroundTool extends BackgroundTool {
46-
type: BackgroundToolType.BROWSER;
47-
metadata: {
48-
url?: string;
49-
error?: string;
50-
};
51-
}
52-
5342
// Agent process specific data (for future use)
5443
export interface AgentBackgroundTool extends BackgroundTool {
5544
type: BackgroundToolType.AGENT;
@@ -60,10 +49,7 @@ export interface AgentBackgroundTool extends BackgroundTool {
6049
}
6150

6251
// Utility type for all background tool types
63-
export type AnyBackgroundTool =
64-
| ShellBackgroundTool
65-
| BrowserBackgroundTool
66-
| AgentBackgroundTool;
52+
export type AnyBackgroundTool = ShellBackgroundTool | AgentBackgroundTool;
6753

6854
/**
6955
* Registry to keep track of all background processes
@@ -90,22 +76,6 @@ export class BackgroundTools {
9076
return id;
9177
}
9278

93-
// Register a new browser process
94-
public registerBrowser(url?: string): string {
95-
const id = uuidv4();
96-
const tool: BrowserBackgroundTool = {
97-
id,
98-
type: BackgroundToolType.BROWSER,
99-
status: BackgroundToolStatus.RUNNING,
100-
startTime: new Date(),
101-
metadata: {
102-
url,
103-
},
104-
};
105-
this.tools.set(id, tool);
106-
return id;
107-
}
108-
10979
// Register a new agent process (for future use)
11080
public registerAgent(goal?: string): string {
11181
const id = uuidv4();
@@ -171,12 +141,6 @@ export class BackgroundTools {
171141
const tools = this.getTools();
172142

173143
// Group tools by type for better cleanup organization
174-
const browserTools = tools.filter(
175-
(tool): tool is BrowserBackgroundTool =>
176-
tool.type === BackgroundToolType.BROWSER &&
177-
tool.status === BackgroundToolStatus.RUNNING,
178-
);
179-
180144
const shellTools = tools.filter(
181145
(tool): tool is ShellBackgroundTool =>
182146
tool.type === BackgroundToolType.SHELL &&
@@ -190,9 +154,6 @@ export class BackgroundTools {
190154
);
191155

192156
// Create cleanup promises for each resource type
193-
const browserCleanupPromises = browserTools.map((tool) =>
194-
this.cleanupBrowserSession(tool),
195-
);
196157
const shellCleanupPromises = shellTools.map((tool) =>
197158
this.cleanupShellProcess(tool),
198159
);
@@ -201,33 +162,7 @@ export class BackgroundTools {
201162
);
202163

203164
// Wait for all cleanup operations to complete in parallel
204-
await Promise.all([
205-
...browserCleanupPromises,
206-
...shellCleanupPromises,
207-
...agentCleanupPromises,
208-
]);
209-
}
210-
211-
/**
212-
* Cleans up a browser session
213-
* @param tool The browser tool to clean up
214-
*/
215-
private async cleanupBrowserSession(
216-
tool: BrowserBackgroundTool,
217-
): Promise<void> {
218-
try {
219-
const browserManager = (
220-
globalThis as unknown as { __BROWSER_MANAGER__?: BrowserManager }
221-
).__BROWSER_MANAGER__;
222-
if (browserManager) {
223-
await browserManager.closeSession(tool.id);
224-
}
225-
this.updateToolStatus(tool.id, BackgroundToolStatus.COMPLETED);
226-
} catch (error) {
227-
this.updateToolStatus(tool.id, BackgroundToolStatus.ERROR, {
228-
error: error instanceof Error ? error.message : String(error),
229-
});
230-
}
165+
await Promise.all([...shellCleanupPromises, ...agentCleanupPromises]);
231166
}
232167

233168
/**

packages/agent/src/core/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export type ToolContext = {
2323
tokenCache?: boolean;
2424
userPrompt?: boolean;
2525
agentId?: string; // Unique identifier for the agent, used for background tool tracking
26+
agentName?: string; // Name of the agent, used for browser tracker
2627
provider: ModelProvider;
2728
model?: string;
2829
baseUrl?: string;

packages/agent/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export * from './tools/browser/browseMessage.js';
1818
export * from './tools/browser/browseStart.js';
1919
export * from './tools/browser/PageController.js';
2020
export * from './tools/browser/BrowserAutomation.js';
21+
export * from './tools/browser/listBrowsers.js';
22+
export * from './tools/browser/browserTracker.js';
2123

2224
// Tools - Interaction
2325
export * from './tools/interaction/subAgent.js';

packages/agent/src/tools/browser/browseMessage.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { z } from 'zod';
22
import { zodToJsonSchema } from 'zod-to-json-schema';
33

4-
import { BackgroundToolStatus } from '../../core/backgroundTools.js';
54
import { Tool } from '../../core/types.js';
65
import { errorToString } from '../../utils/errorToString.js';
76
import { sleep } from '../../utils/sleep.js';
87

8+
import { BrowserSessionStatus, getBrowserTracker } from './browserTracker.js';
99
import { filterPageContent } from './filterPageContent.js';
1010
import { browserSessions, SelectorType } from './types.js';
1111

@@ -72,7 +72,7 @@ export const browseMessageTool: Tool<Parameters, ReturnType> = {
7272

7373
execute: async (
7474
{ instanceId, actionType, url, selector, selectorType, text },
75-
{ logger, pageFilter, backgroundTools },
75+
{ logger, pageFilter, agentName = 'default' },
7676
): Promise<ReturnType> => {
7777
// Validate action format
7878

@@ -87,6 +87,9 @@ export const browseMessageTool: Tool<Parameters, ReturnType> = {
8787
logger.verbose(`Executing browser action: ${actionType}`);
8888
logger.verbose(`Webpage processing mode: ${pageFilter}`);
8989

90+
// Get the browser tracker instance
91+
const browserTracker = getBrowserTracker(agentName);
92+
9093
try {
9194
const session = browserSessions.get(instanceId);
9295
if (!session) {
@@ -186,10 +189,10 @@ export const browseMessageTool: Tool<Parameters, ReturnType> = {
186189
await session.browser.close();
187190
browserSessions.delete(instanceId);
188191

189-
// Update background tool registry when browser is explicitly closed
190-
backgroundTools.updateToolStatus(
192+
// Update browser tracker when browser is explicitly closed
193+
browserTracker.updateSessionStatus(
191194
instanceId,
192-
BackgroundToolStatus.COMPLETED,
195+
BrowserSessionStatus.COMPLETED,
193196
{
194197
closedExplicitly: true,
195198
},
@@ -206,11 +209,15 @@ export const browseMessageTool: Tool<Parameters, ReturnType> = {
206209
} catch (error) {
207210
logger.error('Browser action failed:', { error });
208211

209-
// Update background tool registry with error status if action fails
210-
backgroundTools.updateToolStatus(instanceId, BackgroundToolStatus.ERROR, {
211-
error: errorToString(error),
212-
actionType,
213-
});
212+
// Update browser tracker with error status if action fails
213+
browserTracker.updateSessionStatus(
214+
instanceId,
215+
BrowserSessionStatus.ERROR,
216+
{
217+
error: errorToString(error),
218+
actionType,
219+
},
220+
);
214221

215222
return {
216223
status: 'error',

0 commit comments

Comments
 (0)