From 0787d7b2c5d81e21e563d285c19afafa43d1fd65 Mon Sep 17 00:00:00 2001 From: Nikhil Date: Tue, 4 Nov 2025 22:52:45 +0000 Subject: [PATCH] browseros fixes: loggicodex sdk" (#48) * fix: LLM config for agent * logger: make JSON pretty print * fix: updating logging to be clean and concise --- packages/agent/src/agent/BaseAgent.ts | 5 +- .../agent/src/agent/CodexSDKAgent.config.ts | 6 +- packages/agent/src/agent/CodexSDKAgent.ts | 7 +- packages/agent/src/agent/types.ts | 6 +- packages/agent/src/session/SessionManager.ts | 40 +++++----- packages/common/src/logger.ts | 4 +- packages/server/src/main.ts | 76 +++++++++---------- 7 files changed, 68 insertions(+), 76 deletions(-) diff --git a/packages/agent/src/agent/BaseAgent.ts b/packages/agent/src/agent/BaseAgent.ts index 259b6de39..a88daed5c 100644 --- a/packages/agent/src/agent/BaseAgent.ts +++ b/packages/agent/src/agent/BaseAgent.ts @@ -68,10 +68,11 @@ export abstract class BaseAgent { // Merge config with agent-specific defaults, then with base defaults this.config = { resourcesDir: config.resourcesDir, + executionDir: config.executionDir, mcpServerPort: config.mcpServerPort ?? agentDefaults?.mcpServerPort, apiKey: config.apiKey ?? agentDefaults?.apiKey, - baseUrl: config.baseUrl ?? agentDefaults?.baseUrl, - modelName: config.modelName ?? agentDefaults?.modelName, + baseUrl: config.baseUrl, + modelName: config.modelName, maxTurns: config.maxTurns ?? agentDefaults?.maxTurns ?? DEFAULT_CONFIG.maxTurns, maxThinkingTokens: diff --git a/packages/agent/src/agent/CodexSDKAgent.config.ts b/packages/agent/src/agent/CodexSDKAgent.config.ts index 9ef107039..2c2da815e 100644 --- a/packages/agent/src/agent/CodexSDKAgent.config.ts +++ b/packages/agent/src/agent/CodexSDKAgent.config.ts @@ -17,7 +17,7 @@ export interface McpServerConfig { export interface BrowserOSCodexConfig { model_name: string; - base_url: string; + base_url?: string; api_key_env: string; wire_api: 'chat' | 'responses'; base_instructions_file: string; @@ -26,10 +26,6 @@ export interface BrowserOSCodexConfig { }; } -export function getResourcesDir(resourcesDir?: string): string { - return resourcesDir || process.cwd(); -} - export function generateBrowserOSCodexToml( config: BrowserOSCodexConfig, ): string { diff --git a/packages/agent/src/agent/CodexSDKAgent.ts b/packages/agent/src/agent/CodexSDKAgent.ts index 4f1a1c4f8..dc10ac009 100644 --- a/packages/agent/src/agent/CodexSDKAgent.ts +++ b/packages/agent/src/agent/CodexSDKAgent.ts @@ -16,7 +16,6 @@ import {BaseAgent} from './BaseAgent.js'; import {CodexEventFormatter} from './CodexSDKAgent.formatter.js'; import { type BrowserOSCodexConfig, - getResourcesDir, writeBrowserOSCodexConfig, writePromptFile, } from './CodexSDKAgent.config.js'; @@ -109,14 +108,14 @@ export class CodexSDKAgent extends BaseAgent { } private generateCodexConfig(): void { - const outputDir = getResourcesDir(this.config.executionDir); + const outputDir = this.config.executionDir; const port = this.config.mcpServerPort || CODEX_SDK_DEFAULTS.mcpServerPort; - const modelName = this.config.modelName || 'o4-mini'; + const modelName = this.config.modelName; const baseUrl = this.config.baseUrl; const codexConfig: BrowserOSCodexConfig = { model_name: modelName, - base_url: baseUrl, + ...(baseUrl && {base_url: baseUrl}), api_key_env: 'BROWSEROS_API_KEY', wire_api: 'chat', base_instructions_file: 'browseros_prompt.md', diff --git a/packages/agent/src/agent/types.ts b/packages/agent/src/agent/types.ts index 44d5d0a4b..c6fb175cf 100644 --- a/packages/agent/src/agent/types.ts +++ b/packages/agent/src/agent/types.ts @@ -76,15 +76,13 @@ export const AgentConfigSchema = z.object({ /** * Base URL for custom LLM endpoints - * Optional - used for self-hosted or alternative LLM providers */ - baseUrl: z.string().url().optional(), + baseUrl: z.string().url(), /** * Model name/identifier to use - * Optional - defaults to agent-specific models (e.g., 'o4-mini', 'claude-3-5-sonnet') */ - modelName: z.string().optional(), + modelName: z.string(), /** * Maximum conversation turns before stopping diff --git a/packages/agent/src/session/SessionManager.ts b/packages/agent/src/session/SessionManager.ts index 0b514881f..2e360566b 100644 --- a/packages/agent/src/session/SessionManager.ts +++ b/packages/agent/src/session/SessionManager.ts @@ -92,7 +92,7 @@ export class SessionManager { this.config = config; this.controllerBridge = controllerBridge; - logger.info('๐Ÿ“ฆ SessionManager initialized', { + logger.info('SessionManager initialized', { maxSessions: config.maxSessions, idleTimeoutMs: config.idleTimeoutMs, sharedControllerBridge: true, @@ -145,7 +145,7 @@ export class SessionManager { ); this.agents.set(sessionId, agent); - logger.info('โœ… Session created with agent', { + logger.info('Session created with agent', { sessionId, agentType, totalSessions: this.sessions.size, @@ -154,7 +154,7 @@ export class SessionManager { // Cleanup session if agent creation fails this.sessions.delete(sessionId); - logger.error('โŒ Failed to create agent for session', { + logger.error('Failed to create agent for session', { sessionId, error: error instanceof Error ? error.message : String(error), }); @@ -162,7 +162,7 @@ export class SessionManager { throw error; } } else { - logger.info('โœ… Session created without agent', { + logger.info('Session created without agent', { sessionId, totalSessions: this.sessions.size, }); @@ -201,7 +201,7 @@ export class SessionManager { updateActivity(sessionId: string): void { const session = this.sessions.get(sessionId); if (!session) { - logger.warn('โš ๏ธ Attempted to update activity for non-existent session', { + logger.warn('Attempted to update activity for non-existent session', { sessionId, }); return; @@ -209,7 +209,7 @@ export class SessionManager { session.lastActivity = Date.now(); - logger.debug('๐Ÿ”„ Session activity updated', { + logger.debug('Session activity updated', { sessionId, messageCount: session.messageCount, }); @@ -227,7 +227,7 @@ export class SessionManager { // Reject if already processing (prevent concurrent message handling) if (session.state === SessionState.PROCESSING) { - logger.warn('โš ๏ธ Session already processing message', {sessionId}); + logger.warn('Session already processing message', {sessionId}); return false; } @@ -236,7 +236,7 @@ export class SessionManager { // โŒ Removed: session.lastActivity = Date.now() // Idle timer starts from markIdle(), not here - logger.debug('โš™๏ธ Session marked as processing', { + logger.debug('Session marked as processing', { sessionId, messageCount: session.messageCount, }); @@ -257,7 +257,7 @@ export class SessionManager { session.state = SessionState.IDLE; session.lastActivity = Date.now(); // โœ… Idle timer starts here - logger.debug('๐Ÿ’ค Session marked as idle', {sessionId}); + logger.debug('Session marked as idle', {sessionId}); } /** @@ -280,9 +280,9 @@ export class SessionManager { try { await agent.destroy(); this.agents.delete(sessionId); - logger.debug('๐Ÿ—‘๏ธ Agent destroyed', {sessionId}); + logger.debug('Agent destroyed', {sessionId}); } catch (error) { - logger.error('โŒ Failed to destroy agent', { + logger.error('Failed to destroy agent', { sessionId, error: error instanceof Error ? error.message : String(error), }); @@ -293,7 +293,7 @@ export class SessionManager { // Delete session this.sessions.delete(sessionId); - logger.info('๐Ÿ—‘๏ธ Session deleted', { + logger.info('Session deleted', { sessionId, remainingSessions: this.sessions.size, messageCount: session.messageCount, @@ -341,7 +341,7 @@ export class SessionManager { ) { idleSessionIds.push(sessionId); - logger.info('โฑ๏ธ Idle session detected', { + logger.info('Idle session detected', { sessionId, idleTimeMs: idleTime, threshold: this.config.idleTimeoutMs, @@ -358,17 +358,17 @@ export class SessionManager { */ startCleanup(intervalMs = 60000): () => void { if (this.cleanupTimerId) { - logger.warn('โš ๏ธ Cleanup timer already running'); + logger.warn('Cleanup timer already running'); return () => {}; } - logger.info('๐Ÿงน Starting periodic session cleanup', {intervalMs}); + logger.info('Starting periodic session cleanup', {intervalMs}); this.cleanupTimerId = setInterval(() => { const idleSessionIds = this.findIdleSessions(); if (idleSessionIds.length > 0) { - logger.info('๐Ÿงน Cleanup found idle sessions', { + logger.info('Cleanup found idle sessions', { count: idleSessionIds.length, sessionIds: idleSessionIds, }); @@ -383,7 +383,7 @@ export class SessionManager { if (this.cleanupTimerId) { clearInterval(this.cleanupTimerId); this.cleanupTimerId = undefined; - logger.info('๐Ÿ›‘ Session cleanup stopped'); + logger.info('Session cleanup stopped'); } }; } @@ -429,7 +429,7 @@ export class SessionManager { * Now async to support agent cleanup */ async shutdown(): Promise { - logger.info('๐Ÿ›‘ SessionManager shutting down', { + logger.info('SessionManager shutting down', { activeSessions: this.sessions.size, activeAgents: this.agents.size, }); @@ -445,7 +445,7 @@ export class SessionManager { for (const [sessionId, agent] of this.agents) { destroyPromises.push( agent.destroy().catch(error => { - logger.error('โŒ Failed to destroy agent during shutdown', { + logger.error('Failed to destroy agent during shutdown', { sessionId, error: error instanceof Error ? error.message : String(error), }); @@ -459,6 +459,6 @@ export class SessionManager { // Clear all sessions this.sessions.clear(); - logger.info('โœ… SessionManager shutdown complete'); + logger.info('SessionManager shutdown complete'); } } diff --git a/packages/common/src/logger.ts b/packages/common/src/logger.ts index 9b645c942..da771b84d 100644 --- a/packages/common/src/logger.ts +++ b/packages/common/src/logger.ts @@ -31,13 +31,13 @@ class Logger { private format(level: LogLevel, message: string, meta?: object): string { const timestamp = new Date().toISOString(); const color = COLORS[level]; - const metaStr = meta ? ` ${JSON.stringify(meta)}` : ''; + const metaStr = meta ? `\n${JSON.stringify(meta, null, 2)}` : ''; return `${color}[${timestamp}] [${level.toUpperCase()}]${RESET} ${message}${metaStr}`; } private formatPlain(level: LogLevel, message: string, meta?: object): string { const timestamp = new Date().toISOString(); - const metaStr = meta ? ` ${JSON.stringify(meta)}` : ''; + const metaStr = meta ? `\n${JSON.stringify(meta, null, 2)}` : ''; return `[${timestamp}] [${level.toUpperCase()}] ${message}${metaStr}`; } diff --git a/packages/server/src/main.ts b/packages/server/src/main.ts index 60ac84aa4..4032e116c 100644 --- a/packages/server/src/main.ts +++ b/packages/server/src/main.ts @@ -180,66 +180,64 @@ function startMcpServer(config: { return mcpServer; } -/** - * Get LLM configuration - either all env vars OR all config values (no mixing) - * Environment variables take precedence: if any env var is set, use all env vars - * Otherwise, fetch and use 'default' provider from BROWSEROS_CONFIG_URL - */ +// get LLM configuration for agent server async function getLLMConfig(): Promise<{ apiKey?: string; - baseUrl?: string; - modelName?: string; + baseUrl: string; + modelName: string; }> { - // Check if any environment variable is set const envApiKey = process.env.BROWSEROS_API_KEY; const envBaseUrl = process.env.BROWSEROS_LLM_BASE_URL; const envModelName = process.env.BROWSEROS_LLM_MODEL_NAME; - const hasAnyEnvVar = - envApiKey !== undefined || - envBaseUrl !== undefined || - envModelName !== undefined; - // If any env var is set, use all env vars (no mixing with config) - if (hasAnyEnvVar) { - logger.info('โœ… Using LLM config from environment variables'); - return { - apiKey: envApiKey, - baseUrl: envBaseUrl, - modelName: envModelName, - }; - } + let configApiKey: string | undefined; + let configBaseUrl: string | undefined; + let configModelName: string | undefined; - // No env vars set, try to fetch from config URL + // Try to fetch from config URL const configUrl = process.env.BROWSEROS_CONFIG_URL; if (configUrl) { try { - logger.info('๐ŸŒ Fetching LLM config from BrowserOS Config URL', { + logger.info('Fetching LLM config from BrowserOS Config URL', { configUrl, }); const config = await fetchBrowserOSConfig(configUrl); const llmConfig = getLLMConfigFromProvider(config, 'default'); - logger.info('โœ… Using LLM config from BrowserOS Config (default provider)'); - return { - apiKey: llmConfig.apiKey, - baseUrl: llmConfig.baseUrl, - modelName: llmConfig.modelName, - }; + configApiKey = llmConfig.apiKey; + configBaseUrl = llmConfig.baseUrl; + configModelName = llmConfig.modelName; + + logger.info('Loaded config from BrowserOS Config (default provider)'); } catch (error) { - logger.warn( - 'โš ๏ธ Failed to fetch config from URL, no LLM config available', - { - error: error instanceof Error ? error.message : String(error), - }, - ); + logger.warn('Failed to fetch config from URL', { + error: error instanceof Error ? error.message : String(error), + }); } } - // No env vars and no config available + // Apply env var overrides (env takes precedence) + const apiKey = envApiKey ?? configApiKey; + const baseUrl = envBaseUrl ?? configBaseUrl; + const modelName = envModelName ?? configModelName; + + // Validate required fields + if (!baseUrl || !modelName) { + throw new Error( + 'LLM configuration required: baseUrl and modelName must be set via BROWSEROS_LLM_BASE_URL and BROWSEROS_LLM_MODEL_NAME environment variables, or via BROWSEROS_CONFIG_URL', + ); + } + + logger.info('Using LLM config', { + baseUrl, + modelName, + apiKeySource: envApiKey ? 'env' : configApiKey ? 'config' : 'none', + }); + return { - apiKey: undefined, - baseUrl: undefined, - modelName: undefined, + apiKey, + baseUrl, + modelName, }; }