Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 1 addition & 74 deletions packages/agent/src/core/backgroundTools.cleanup.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';

// Import mocked modules
import { BrowserManager } from '../tools/browser/BrowserManager.js';
import { agentStates } from '../tools/interaction/agentStart.js';
import { processStates } from '../tools/system/shellStart.js';

Expand Down Expand Up @@ -35,14 +34,6 @@
};

// Mock dependencies
vi.mock('../tools/browser/BrowserManager.js', () => {
return {
BrowserManager: class MockBrowserManager {
closeSession = vi.fn().mockResolvedValue(undefined);
},
};
});

vi.mock('../tools/system/shellStart.js', () => {
return {
processStates: new Map<string, MockProcessState>(),
Expand All @@ -58,20 +49,13 @@
describe('BackgroundTools cleanup', () => {
let backgroundTools: BackgroundTools;

// Setup mocks for globalThis and process states
// Setup mocks for process states
beforeEach(() => {
backgroundTools = new BackgroundTools('test-agent');

// Reset mocks
vi.resetAllMocks();

// Setup global browser manager
(
globalThis as unknown as { __BROWSER_MANAGER__: BrowserManager }
).__BROWSER_MANAGER__ = {
closeSession: vi.fn().mockResolvedValue(undefined),
} as unknown as BrowserManager;

// Setup mock process states
const mockProcess = {
kill: vi.fn(),
Expand All @@ -88,7 +72,7 @@
};

processStates.clear();
processStates.set('shell-1', mockProcessState as any);

Check warning on line 75 in packages/agent/src/core/backgroundTools.cleanup.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

Check warning on line 75 in packages/agent/src/core/backgroundTools.cleanup.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

// Setup mock agent states
const mockAgentState: MockAgentState = {
Expand All @@ -107,40 +91,17 @@
};

agentStates.clear();
agentStates.set('agent-1', mockAgentState as any);

Check warning on line 94 in packages/agent/src/core/backgroundTools.cleanup.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

Check warning on line 94 in packages/agent/src/core/backgroundTools.cleanup.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type
});

afterEach(() => {
vi.resetAllMocks();

// Clear global browser manager
(
globalThis as unknown as { __BROWSER_MANAGER__?: BrowserManager }
).__BROWSER_MANAGER__ = undefined;

// Clear mock states
processStates.clear();
agentStates.clear();
});

it('should clean up browser sessions', async () => {
// Register a browser tool
const browserId = backgroundTools.registerBrowser('https://example.com');

// Run cleanup
await backgroundTools.cleanup();

// Check that closeSession was called
expect(
(globalThis as unknown as { __BROWSER_MANAGER__: BrowserManager })
.__BROWSER_MANAGER__.closeSession,
).toHaveBeenCalledWith(browserId);

// Check that tool status was updated
const tool = backgroundTools.getToolById(browserId);
expect(tool?.status).toBe(BackgroundToolStatus.COMPLETED);
});

it('should clean up shell processes', async () => {
// Register a shell tool
const shellId = backgroundTools.registerShell('echo "test"');
Expand All @@ -149,7 +110,7 @@
const mockProcessState = processStates.get('shell-1');

// Set the shell ID to match
processStates.set(shellId, processStates.get('shell-1') as any);

Check warning on line 113 in packages/agent/src/core/backgroundTools.cleanup.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

Check warning on line 113 in packages/agent/src/core/backgroundTools.cleanup.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

// Run cleanup
await backgroundTools.cleanup();
Expand All @@ -170,7 +131,7 @@
const mockAgentState = agentStates.get('agent-1');

// Set the agent ID to match
agentStates.set(agentId, agentStates.get('agent-1') as any);

Check warning on line 134 in packages/agent/src/core/backgroundTools.cleanup.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

Check warning on line 134 in packages/agent/src/core/backgroundTools.cleanup.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

// Run cleanup
await backgroundTools.cleanup();
Expand All @@ -186,38 +147,4 @@
const tool = backgroundTools.getToolById(agentId);
expect(tool?.status).toBe(BackgroundToolStatus.TERMINATED);
});

it('should handle errors during cleanup', async () => {
// Register a browser tool
const browserId = backgroundTools.registerBrowser('https://example.com');

// Make closeSession throw an error
(
(globalThis as unknown as { __BROWSER_MANAGER__: BrowserManager })
.__BROWSER_MANAGER__.closeSession as ReturnType<typeof vi.fn>
).mockRejectedValue(new Error('Test error'));

// Run cleanup
await backgroundTools.cleanup();

// Check that tool status was updated to ERROR
const tool = backgroundTools.getToolById(browserId);
expect(tool?.status).toBe(BackgroundToolStatus.ERROR);
expect(tool?.metadata.error).toBe('Test error');
});

it('should only clean up running tools', async () => {
// Register a browser tool and mark it as completed
const browserId = backgroundTools.registerBrowser('https://example.com');
backgroundTools.updateToolStatus(browserId, BackgroundToolStatus.COMPLETED);

// Run cleanup
await backgroundTools.cleanup();

// Check that closeSession was not called
expect(
(globalThis as unknown as { __BROWSER_MANAGER__: BrowserManager })
.__BROWSER_MANAGER__.closeSession,
).not.toHaveBeenCalled();
});
});
16 changes: 1 addition & 15 deletions packages/agent/src/core/backgroundTools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,7 @@
}
});

it('should register a browser process', () => {
const id = backgroundTools.registerBrowser('https://example.com');

expect(id).toBe('test-id-1');

const tool = backgroundTools.getToolById(id);
expect(tool).toBeDefined();
if (tool) {
expect(tool.type).toBe(BackgroundToolType.BROWSER);
expect(tool.status).toBe(BackgroundToolStatus.RUNNING);
if (tool.type === BackgroundToolType.BROWSER) {
expect(tool.metadata.url).toBe('https://example.com');
}
}
});
// Browser registration test removed since browser tracking is now decoupled

it('should update tool status', () => {
const id = backgroundTools.registerShell('sleep 10');
Expand Down Expand Up @@ -86,7 +72,7 @@

it('should clean up old completed tools', () => {
// Create tools with specific dates
const registry = backgroundTools as any;

Check warning on line 75 in packages/agent/src/core/backgroundTools.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

Check warning on line 75 in packages/agent/src/core/backgroundTools.test.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

// Add a completed tool from 25 hours ago
const oldTool = {
Expand Down
69 changes: 2 additions & 67 deletions packages/agent/src/core/backgroundTools.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import { v4 as uuidv4 } from 'uuid';

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

// Types of background processes we can track
export enum BackgroundToolType {
SHELL = 'shell',
BROWSER = 'browser',
AGENT = 'agent',
}

Expand All @@ -27,7 +25,7 @@
status: BackgroundToolStatus;
startTime: Date;
endTime?: Date;
metadata: Record<string, any>; // Additional tool-specific information

Check warning on line 28 in packages/agent/src/core/backgroundTools.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

Check warning on line 28 in packages/agent/src/core/backgroundTools.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type
}

// Shell process specific data
Expand All @@ -41,15 +39,6 @@
};
}

// Browser process specific data
export interface BrowserBackgroundTool extends BackgroundTool {
type: BackgroundToolType.BROWSER;
metadata: {
url?: string;
error?: string;
};
}

// Agent process specific data (for future use)
export interface AgentBackgroundTool extends BackgroundTool {
type: BackgroundToolType.AGENT;
Expand All @@ -60,10 +49,7 @@
}

// Utility type for all background tool types
export type AnyBackgroundTool =
| ShellBackgroundTool
| BrowserBackgroundTool
| AgentBackgroundTool;
export type AnyBackgroundTool = ShellBackgroundTool | AgentBackgroundTool;

/**
* Registry to keep track of all background processes
Expand All @@ -90,22 +76,6 @@
return id;
}

// Register a new browser process
public registerBrowser(url?: string): string {
const id = uuidv4();
const tool: BrowserBackgroundTool = {
id,
type: BackgroundToolType.BROWSER,
status: BackgroundToolStatus.RUNNING,
startTime: new Date(),
metadata: {
url,
},
};
this.tools.set(id, tool);
return id;
}

// Register a new agent process (for future use)
public registerAgent(goal?: string): string {
const id = uuidv4();
Expand All @@ -126,7 +96,7 @@
public updateToolStatus(
id: string,
status: BackgroundToolStatus,
metadata?: Record<string, any>,

Check warning on line 99 in packages/agent/src/core/backgroundTools.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type

Check warning on line 99 in packages/agent/src/core/backgroundTools.ts

View workflow job for this annotation

GitHub Actions / ci

Unexpected any. Specify a different type
): boolean {
const tool = this.tools.get(id);
if (!tool) {
Expand Down Expand Up @@ -171,12 +141,6 @@
const tools = this.getTools();

// Group tools by type for better cleanup organization
const browserTools = tools.filter(
(tool): tool is BrowserBackgroundTool =>
tool.type === BackgroundToolType.BROWSER &&
tool.status === BackgroundToolStatus.RUNNING,
);

const shellTools = tools.filter(
(tool): tool is ShellBackgroundTool =>
tool.type === BackgroundToolType.SHELL &&
Expand All @@ -190,9 +154,6 @@
);

// Create cleanup promises for each resource type
const browserCleanupPromises = browserTools.map((tool) =>
this.cleanupBrowserSession(tool),
);
const shellCleanupPromises = shellTools.map((tool) =>
this.cleanupShellProcess(tool),
);
Expand All @@ -201,33 +162,7 @@
);

// Wait for all cleanup operations to complete in parallel
await Promise.all([
...browserCleanupPromises,
...shellCleanupPromises,
...agentCleanupPromises,
]);
}

/**
* Cleans up a browser session
* @param tool The browser tool to clean up
*/
private async cleanupBrowserSession(
tool: BrowserBackgroundTool,
): Promise<void> {
try {
const browserManager = (
globalThis as unknown as { __BROWSER_MANAGER__?: BrowserManager }
).__BROWSER_MANAGER__;
if (browserManager) {
await browserManager.closeSession(tool.id);
}
this.updateToolStatus(tool.id, BackgroundToolStatus.COMPLETED);
} catch (error) {
this.updateToolStatus(tool.id, BackgroundToolStatus.ERROR, {
error: error instanceof Error ? error.message : String(error),
});
}
await Promise.all([...shellCleanupPromises, ...agentCleanupPromises]);
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/agent/src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export type ToolContext = {
tokenCache?: boolean;
userPrompt?: boolean;
agentId?: string; // Unique identifier for the agent, used for background tool tracking
agentName?: string; // Name of the agent, used for browser tracker
provider: ModelProvider;
model?: string;
baseUrl?: string;
Expand Down
2 changes: 2 additions & 0 deletions packages/agent/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export * from './tools/browser/browseMessage.js';
export * from './tools/browser/browseStart.js';
export * from './tools/browser/PageController.js';
export * from './tools/browser/BrowserAutomation.js';
export * from './tools/browser/listBrowsers.js';
export * from './tools/browser/browserTracker.js';

// Tools - Interaction
export * from './tools/interaction/subAgent.js';
Expand Down
27 changes: 17 additions & 10 deletions packages/agent/src/tools/browser/browseMessage.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { z } from 'zod';
import { zodToJsonSchema } from 'zod-to-json-schema';

import { BackgroundToolStatus } from '../../core/backgroundTools.js';
import { Tool } from '../../core/types.js';
import { errorToString } from '../../utils/errorToString.js';
import { sleep } from '../../utils/sleep.js';

import { BrowserSessionStatus, getBrowserTracker } from './browserTracker.js';
import { filterPageContent } from './filterPageContent.js';
import { browserSessions, SelectorType } from './types.js';

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

execute: async (
{ instanceId, actionType, url, selector, selectorType, text },
{ logger, pageFilter, backgroundTools },
{ logger, pageFilter, agentName = 'default' },
): Promise<ReturnType> => {
// Validate action format

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

// Get the browser tracker instance
const browserTracker = getBrowserTracker(agentName);

try {
const session = browserSessions.get(instanceId);
if (!session) {
Expand Down Expand Up @@ -186,10 +189,10 @@ export const browseMessageTool: Tool<Parameters, ReturnType> = {
await session.browser.close();
browserSessions.delete(instanceId);

// Update background tool registry when browser is explicitly closed
backgroundTools.updateToolStatus(
// Update browser tracker when browser is explicitly closed
browserTracker.updateSessionStatus(
instanceId,
BackgroundToolStatus.COMPLETED,
BrowserSessionStatus.COMPLETED,
{
closedExplicitly: true,
},
Expand All @@ -206,11 +209,15 @@ export const browseMessageTool: Tool<Parameters, ReturnType> = {
} catch (error) {
logger.error('Browser action failed:', { error });

// Update background tool registry with error status if action fails
backgroundTools.updateToolStatus(instanceId, BackgroundToolStatus.ERROR, {
error: errorToString(error),
actionType,
});
// Update browser tracker with error status if action fails
browserTracker.updateSessionStatus(
instanceId,
BrowserSessionStatus.ERROR,
{
error: errorToString(error),
actionType,
},
);

return {
status: 'error',
Expand Down
Loading
Loading