From 5dedfe93ec8551607708a0ab537ce03740d34698 Mon Sep 17 00:00:00 2001 From: "Ben Houston (via MyCoder)" Date: Fri, 14 Mar 2025 19:50:35 +0000 Subject: [PATCH] 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 --- .../src/core/backgroundTools.cleanup.test.ts | 75 +------- .../agent/src/core/backgroundTools.test.ts | 16 +- packages/agent/src/core/backgroundTools.ts | 69 +------- packages/agent/src/core/types.ts | 1 + packages/agent/src/index.ts | 2 + .../agent/src/tools/browser/browseMessage.ts | 27 +-- .../agent/src/tools/browser/browseStart.ts | 26 +-- .../agent/src/tools/browser/browserTracker.ts | 160 ++++++++++++++++++ .../agent/src/tools/browser/listBrowsers.ts | 105 ++++++++++++ .../src/tools/system/listBackgroundTools.ts | 5 +- 10 files changed, 304 insertions(+), 182 deletions(-) create mode 100644 packages/agent/src/tools/browser/browserTracker.ts create mode 100644 packages/agent/src/tools/browser/listBrowsers.ts diff --git a/packages/agent/src/core/backgroundTools.cleanup.test.ts b/packages/agent/src/core/backgroundTools.cleanup.test.ts index 3adec5d..6ca9c28 100644 --- a/packages/agent/src/core/backgroundTools.cleanup.test.ts +++ b/packages/agent/src/core/backgroundTools.cleanup.test.ts @@ -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'; @@ -35,14 +34,6 @@ type MockAgentState = { }; // 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(), @@ -58,20 +49,13 @@ vi.mock('../tools/interaction/agentStart.js', () => { 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(), @@ -113,34 +97,11 @@ describe('BackgroundTools cleanup', () => { 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"'); @@ -186,38 +147,4 @@ describe('BackgroundTools cleanup', () => { 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 - ).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(); - }); }); diff --git a/packages/agent/src/core/backgroundTools.test.ts b/packages/agent/src/core/backgroundTools.test.ts index 4b0e5c3..53631a2 100644 --- a/packages/agent/src/core/backgroundTools.test.ts +++ b/packages/agent/src/core/backgroundTools.test.ts @@ -35,21 +35,7 @@ describe('BackgroundToolRegistry', () => { } }); - 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'); diff --git a/packages/agent/src/core/backgroundTools.ts b/packages/agent/src/core/backgroundTools.ts index 45c61c1..5bba713 100644 --- a/packages/agent/src/core/backgroundTools.ts +++ b/packages/agent/src/core/backgroundTools.ts @@ -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', } @@ -41,15 +39,6 @@ export interface ShellBackgroundTool extends BackgroundTool { }; } -// 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; @@ -60,10 +49,7 @@ export interface AgentBackgroundTool extends BackgroundTool { } // 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 @@ -90,22 +76,6 @@ export class BackgroundTools { 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(); @@ -171,12 +141,6 @@ export class BackgroundTools { 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 && @@ -190,9 +154,6 @@ export class BackgroundTools { ); // Create cleanup promises for each resource type - const browserCleanupPromises = browserTools.map((tool) => - this.cleanupBrowserSession(tool), - ); const shellCleanupPromises = shellTools.map((tool) => this.cleanupShellProcess(tool), ); @@ -201,33 +162,7 @@ export class BackgroundTools { ); // 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 { - 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]); } /** diff --git a/packages/agent/src/core/types.ts b/packages/agent/src/core/types.ts index a0a411e..0d386b2 100644 --- a/packages/agent/src/core/types.ts +++ b/packages/agent/src/core/types.ts @@ -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; diff --git a/packages/agent/src/index.ts b/packages/agent/src/index.ts index e0a9567..6bb65d7 100644 --- a/packages/agent/src/index.ts +++ b/packages/agent/src/index.ts @@ -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'; diff --git a/packages/agent/src/tools/browser/browseMessage.ts b/packages/agent/src/tools/browser/browseMessage.ts index a6b35d5..77b94e4 100644 --- a/packages/agent/src/tools/browser/browseMessage.ts +++ b/packages/agent/src/tools/browser/browseMessage.ts @@ -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'; @@ -72,7 +72,7 @@ export const browseMessageTool: Tool = { execute: async ( { instanceId, actionType, url, selector, selectorType, text }, - { logger, pageFilter, backgroundTools }, + { logger, pageFilter, agentName = 'default' }, ): Promise => { // Validate action format @@ -87,6 +87,9 @@ export const browseMessageTool: Tool = { 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) { @@ -186,10 +189,10 @@ export const browseMessageTool: Tool = { 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, }, @@ -206,11 +209,15 @@ export const browseMessageTool: Tool = { } 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', diff --git a/packages/agent/src/tools/browser/browseStart.ts b/packages/agent/src/tools/browser/browseStart.ts index ad41298..d2fc743 100644 --- a/packages/agent/src/tools/browser/browseStart.ts +++ b/packages/agent/src/tools/browser/browseStart.ts @@ -1,13 +1,12 @@ import { chromium } from '@playwright/test'; -import { v4 as uuidv4 } from 'uuid'; 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 } from './types.js'; @@ -43,7 +42,7 @@ export const browseStartTool: Tool = { execute: async ( { url, timeout = 30000 }, - { logger, headless, userSession, pageFilter, backgroundTools }, + { logger, headless, userSession, pageFilter, agentName = 'default' }, ): Promise => { logger.verbose(`Starting browser session${url ? ` at ${url}` : ''}`); logger.verbose( @@ -52,10 +51,11 @@ export const browseStartTool: Tool = { logger.verbose(`Webpage processing mode: ${pageFilter}`); try { - const instanceId = uuidv4(); + // Get the browser tracker instance + const browserTracker = getBrowserTracker(agentName); - // Register this browser session with the background tool registry - backgroundTools.registerBrowser(url); + // Register this browser session with the tracker + const instanceId = browserTracker.registerBrowser(url); // Launch browser const launchOptions = { @@ -95,10 +95,10 @@ export const browseStartTool: Tool = { // Setup cleanup handlers browser.on('disconnected', () => { browserSessions.delete(instanceId); - // Update background tool registry when browser disconnects - backgroundTools.updateToolStatus( + // Update browser tracker when browser disconnects + browserTracker.updateSessionStatus( instanceId, - BackgroundToolStatus.TERMINATED, + BrowserSessionStatus.TERMINATED, ); }); @@ -142,10 +142,10 @@ export const browseStartTool: Tool = { logger.verbose('Browser session started successfully'); logger.verbose(`Content length: ${content.length} characters`); - // Update background tool registry with running status - backgroundTools.updateToolStatus( + // Update browser tracker with running status + browserTracker.updateSessionStatus( instanceId, - BackgroundToolStatus.RUNNING, + BrowserSessionStatus.RUNNING, { url: url || 'about:blank', contentLength: content.length, @@ -160,7 +160,7 @@ export const browseStartTool: Tool = { } catch (error) { logger.error(`Failed to start browser: ${errorToString(error)}`); - // No need to update background tool registry here as we don't have a valid instanceId + // No need to update browser tracker here as we don't have a valid instanceId // when an error occurs before the browser is properly initialized return { diff --git a/packages/agent/src/tools/browser/browserTracker.ts b/packages/agent/src/tools/browser/browserTracker.ts new file mode 100644 index 0000000..94c0217 --- /dev/null +++ b/packages/agent/src/tools/browser/browserTracker.ts @@ -0,0 +1,160 @@ +import { v4 as uuidv4 } from 'uuid'; + +import { BrowserManager } from './BrowserManager.js'; +import { browserSessions } from './types.js'; + +// Status of a browser session +export enum BrowserSessionStatus { + RUNNING = 'running', + COMPLETED = 'completed', + ERROR = 'error', + TERMINATED = 'terminated', +} + +// Browser session tracking data +export interface BrowserSessionInfo { + id: string; + status: BrowserSessionStatus; + startTime: Date; + endTime?: Date; + metadata: { + url?: string; + contentLength?: number; + closedExplicitly?: boolean; + error?: string; + actionType?: string; + }; +} + +/** + * Registry to keep track of browser sessions + */ +export class BrowserTracker { + private sessions: Map = new Map(); + + // Private constructor for singleton pattern + constructor(readonly ownerName: string) {} + + // Register a new browser session + public registerBrowser(url?: string): string { + const id = uuidv4(); + const session: BrowserSessionInfo = { + id, + status: BrowserSessionStatus.RUNNING, + startTime: new Date(), + metadata: { + url, + }, + }; + this.sessions.set(id, session); + return id; + } + + // Update the status of a browser session + public updateSessionStatus( + id: string, + status: BrowserSessionStatus, + metadata?: Record, + ): boolean { + const session = this.sessions.get(id); + if (!session) { + return false; + } + + session.status = status; + + if ( + status === BrowserSessionStatus.COMPLETED || + status === BrowserSessionStatus.ERROR || + status === BrowserSessionStatus.TERMINATED + ) { + session.endTime = new Date(); + } + + if (metadata) { + session.metadata = { ...session.metadata, ...metadata }; + } + + return true; + } + + // Get all browser sessions + public getSessions(): BrowserSessionInfo[] { + return Array.from(this.sessions.values()); + } + + // Get a specific browser session by ID + public getSessionById(id: string): BrowserSessionInfo | undefined { + return this.sessions.get(id); + } + + // Filter sessions by status + public getSessionsByStatus( + status: BrowserSessionStatus, + ): BrowserSessionInfo[] { + return this.getSessions().filter((session) => session.status === status); + } + + /** + * Cleans up all browser sessions associated with this tracker + * @returns A promise that resolves when cleanup is complete + */ + public async cleanup(): Promise { + const sessions = this.getSessionsByStatus(BrowserSessionStatus.RUNNING); + + // Create cleanup promises for each session + const cleanupPromises = sessions.map((session) => + this.cleanupBrowserSession(session), + ); + + // Wait for all cleanup operations to complete in parallel + await Promise.all(cleanupPromises); + } + + /** + * Cleans up a browser session + * @param session The browser session to clean up + */ + private async cleanupBrowserSession( + session: BrowserSessionInfo, + ): Promise { + try { + const browserManager = ( + globalThis as unknown as { __BROWSER_MANAGER__?: BrowserManager } + ).__BROWSER_MANAGER__; + + if (browserManager) { + await browserManager.closeSession(session.id); + } else { + // Fallback to closing via browserSessions if BrowserManager is not available + const browserSession = browserSessions.get(session.id); + if (browserSession) { + await browserSession.page.context().close(); + await browserSession.browser.close(); + browserSessions.delete(session.id); + } + } + + this.updateSessionStatus(session.id, BrowserSessionStatus.COMPLETED); + } catch (error) { + this.updateSessionStatus(session.id, BrowserSessionStatus.ERROR, { + error: error instanceof Error ? error.message : String(error), + }); + } + } +} + +// Create a singleton instance +let browserTracker: BrowserTracker | null = null; + +/** + * Get the singleton instance of the BrowserTracker + * @param ownerName The name of the owner (agent) for this tracker + * @returns The singleton BrowserTracker instance + */ +export function getBrowserTracker(ownerName: string): BrowserTracker { + if (!browserTracker) { + browserTracker = new BrowserTracker(ownerName); + } + return browserTracker; +} diff --git a/packages/agent/src/tools/browser/listBrowsers.ts b/packages/agent/src/tools/browser/listBrowsers.ts new file mode 100644 index 0000000..f231577 --- /dev/null +++ b/packages/agent/src/tools/browser/listBrowsers.ts @@ -0,0 +1,105 @@ +import { z } from 'zod'; +import { zodToJsonSchema } from 'zod-to-json-schema'; + +import { Tool } from '../../core/types.js'; + +import { BrowserSessionStatus, getBrowserTracker } from './browserTracker.js'; + +const parameterSchema = z.object({ + status: z + .enum(['all', 'running', 'completed', 'error', 'terminated']) + .optional() + .describe('Filter browser sessions by status (default: "all")'), + verbose: z + .boolean() + .optional() + .describe( + 'Include detailed metadata about each browser session (default: false)', + ), +}); + +const returnSchema = z.object({ + sessions: z.array( + z.object({ + id: z.string(), + status: z.string(), + startTime: z.string(), + endTime: z.string().optional(), + runtime: z.number().describe('Runtime in seconds'), + url: z.string().optional(), + metadata: z.record(z.any()).optional(), + }), + ), + count: z.number(), +}); + +type Parameters = z.infer; +type ReturnType = z.infer; + +export const listBrowsersTool: Tool = { + name: 'listBrowsers', + description: 'Lists all browser sessions and their status', + logPrefix: '🔍', + parameters: parameterSchema, + returns: returnSchema, + parametersJsonSchema: zodToJsonSchema(parameterSchema), + returnsJsonSchema: zodToJsonSchema(returnSchema), + + execute: async ( + { status = 'all', verbose = false }, + { logger, agentName = 'default' }, + ): Promise => { + logger.verbose( + `Listing browser sessions with status: ${status}, verbose: ${verbose}`, + ); + + // Get the browser tracker instance + const browserTracker = getBrowserTracker(agentName); + + // Get all browser sessions + const sessions = browserTracker.getSessions(); + + // Filter by status if specified + const filteredSessions = + status === 'all' + ? sessions + : sessions.filter((session) => { + const statusEnum = + status.toUpperCase() as keyof typeof BrowserSessionStatus; + return session.status === BrowserSessionStatus[statusEnum]; + }); + + // Format the response + const formattedSessions = filteredSessions.map((session) => { + const now = new Date(); + const startTime = session.startTime; + const endTime = session.endTime || now; + const runtime = (endTime.getTime() - startTime.getTime()) / 1000; // in seconds + + return { + id: session.id, + status: session.status, + startTime: startTime.toISOString(), + ...(session.endTime && { endTime: session.endTime.toISOString() }), + runtime: parseFloat(runtime.toFixed(2)), + url: session.metadata.url, + ...(verbose && { metadata: session.metadata }), + }; + }); + + return { + sessions: formattedSessions, + count: formattedSessions.length, + }; + }, + + logParameters: ({ status = 'all', verbose = false }, { logger }) => { + logger.info( + `Listing browser sessions with status: ${status}, verbose: ${verbose}`, + ); + }, + + logReturns: (output, { logger }) => { + logger.info(`Found ${output.count} browser sessions`); + }, +}; diff --git a/packages/agent/src/tools/system/listBackgroundTools.ts b/packages/agent/src/tools/system/listBackgroundTools.ts index bc7608e..584d4ee 100644 --- a/packages/agent/src/tools/system/listBackgroundTools.ts +++ b/packages/agent/src/tools/system/listBackgroundTools.ts @@ -10,7 +10,7 @@ const parameterSchema = z.object({ .optional() .describe('Filter tools by status (default: "all")'), type: z - .enum(['all', 'shell', 'browser', 'agent']) + .enum(['all', 'shell', 'agent']) .optional() .describe('Filter tools by type (default: "all")'), verbose: z @@ -39,8 +39,7 @@ type ReturnType = z.infer; export const listBackgroundToolsTool: Tool = { name: 'listBackgroundTools', - description: - 'Lists all background tools (shells, browsers, agents) and their status', + description: 'Lists all background tools (shells, agents) and their status', logPrefix: '🔍', parameters: parameterSchema, returns: returnSchema,