Integate browseros config #19
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR introduces dynamic BrowserOS configuration fetching via a new BROWSEROS_CONFIG_URL environment variable. The system now retrieves API keys and model settings from a remote endpoint before falling back to the existing ANTHROPIC_API_KEY environment variable. The implementation adds a new gateway.ts module in the common package that handles HTTP configuration retrieval, exports the functionality through the common package index, and integrates async initialization into the BaseAgent class hierarchy. The agent server startup validation was modified to allow initialization without an API key, deferring validation to when agents actually need credentials.
The changes fit into the existing monorepo structure by centralizing configuration logic in @browseros/common, following established patterns for shared utilities. The async initialization pattern addresses the constraint that JavaScript constructors cannot be async, enabling agents to fetch configuration before execution.
PR Description Notes:
- The PR title contains a typo: "Integate" should be "Integrate"
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| packages/common/src/gateway.ts | 2/5 | New module for fetching BrowserOS config from remote URL with critical issues: no fetch timeout, weak type validation, and potential credential exposure in logs |
| packages/agent/src/agent/ClaudeSDKAgent.ts | 4/5 | Implements config URL fetching with fallback to environment variable, adds lazy initialization to execute(), and allows runtime model override |
| packages/agent/src/agent/BaseAgent.ts | 4/5 | Introduces async initialization pattern with initialized flag and init() method, but includes unresolved FIXME about automatic initialization in execute() |
| packages/server/src/main.ts | 3/5 | Removes startup validation for ANTHROPIC_API_KEY, allowing server to start without API key and deferring errors to runtime |
| .env.example | 3/5 | Adds BROWSEROS_CONFIG_URL variable but removes the Required comment section, causing potential confusion about mandatory variables |
| packages/common/src/index.ts | 5/5 | Clean export addition of fetchBrowserOSConfig function and BrowserOSConfig type following established patterns |
Confidence score: 3/5
- This PR requires careful review due to production-readiness concerns in the core configuration fetching mechanism
- Score reflects critical issues in gateway.ts (no timeout, weak validation, credential logging), removal of fail-fast startup validation, and an unresolved FIXME in BaseAgent that could cause runtime errors if subclasses forget to call ensureInitialized()
- Pay close attention to packages/common/src/gateway.ts which needs timeout handling and proper type validation, and packages/server/src/main.ts where removing API key validation could make debugging harder
Sequence Diagram
sequenceDiagram
participant User
participant Main as main.ts
participant Controller as ControllerBridge
participant CDP as CDP Connection
participant MCP as MCP Server
participant Agent as Agent Server
participant ClaudeAgent as ClaudeSDKAgent
participant Gateway as BrowserOS Config
User->>Main: "Start Server"
Main->>Main: "parseArguments()"
Main->>Controller: "new ControllerBridge(extensionPort)"
Main->>CDP: "ensureBrowserConnected(cdpPort)"
CDP-->>Main: "Browser connection"
Main->>Main: "mergeTools(cdpContext, controllerContext)"
Main->>MCP: "createHttpMcpServer(config)"
MCP-->>Main: "HTTP Server instance"
Main->>Agent: "createAgentServer(agentConfig, controllerBridge)"
Agent-->>Main: "Agent Server instance"
Main->>Main: "logSummary(ports)"
User->>Agent: "WebSocket Connect"
Agent->>ClaudeAgent: "new ClaudeSDKAgent(config, controllerBridge)"
ClaudeAgent->>ClaudeAgent: "init()"
alt BROWSEROS_CONFIG_URL set
ClaudeAgent->>Gateway: "fetchBrowserOSConfig(configUrl)"
Gateway-->>ClaudeAgent: "BrowserOSConfig {model, apiKey}"
ClaudeAgent->>ClaudeAgent: "config.apiKey = gatewayConfig.apiKey"
else Fallback to env var
ClaudeAgent->>ClaudeAgent: "config.apiKey = ANTHROPIC_API_KEY"
end
User->>Agent: "execute(message)"
Agent->>ClaudeAgent: "execute(message)"
ClaudeAgent->>ClaudeAgent: "query({prompt, options})"
loop Stream Events
ClaudeAgent->>ClaudeAgent: "EventFormatter.format(event)"
ClaudeAgent-->>Agent: "FormattedEvent"
Agent-->>User: "WebSocket Event"
end
User->>Main: "SIGINT/SIGTERM"
Main->>MCP: "shutdownMcpServer()"
Main->>Agent: "stop()"
Main->>Controller: "close()"
Main->>Main: "process.exit(0)"
Context used:
- Context from
dashboard- CLAUDE.md (source)
6 files reviewed, 3 comments
| const response = await fetch(configUrl, { | ||
| method: 'GET', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
logic: fetch() call has no timeout or retry logic. If the config URL is slow or unresponsive, this will hang indefinitely and block server startup.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/common/src/gateway.ts
Line: 19:24
Comment:
**logic:** fetch() call has no timeout or retry logic. If the config URL is slow or unresponsive, this will hang indefinitely and block server startup.
How can I resolve this? If you propose a fix, please make it concise.| const config = (await response.json()) as BrowserOSConfig; | ||
|
|
||
| if (!config.model || !config.apiKey) { | ||
| throw new Error('Invalid config response: missing model or apiKey'); | ||
| } |
There was a problem hiding this comment.
logic: Type assertion bypasses validation. The response could contain unexpected data types (e.g., model: 123 instead of string) which would pass this check but fail later.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/common/src/gateway.ts
Line: 33:37
Comment:
**logic:** Type assertion bypasses validation. The response could contain unexpected data types (e.g., `model: 123` instead of string) which would pass this check but fail later.
How can I resolve this? If you propose a fix, please make it concise.| logger.error('❌ Failed to fetch BrowserOS config', { | ||
| configUrl, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); |
There was a problem hiding this comment.
logic: Error log includes the full configUrl which may contain sensitive information like API keys or tokens in query parameters.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/common/src/gateway.ts
Line: 43:46
Comment:
**logic:** Error log includes the full `configUrl` which may contain sensitive information like API keys or tokens in query parameters.
How can I resolve this? If you propose a fix, please make it concise.
No description provided.