Skip to content
Open
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,6 @@ log.txt

# Testing iteration temp files
tmp/

# Coding agent artifacts
.agent/
3 changes: 0 additions & 3 deletions apps/server/src/api/routes/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { zValidator } from '@hono/zod-validator'
import { Hono } from 'hono'
import { stream } from 'hono/streaming'
import { SessionManager } from '../../agent/session'
import { KlavisClient } from '../../lib/clients/klavis/klavis-client'
import { logger } from '../../lib/logger'
import { metrics } from '../../lib/metrics'
import type { RateLimiter } from '../../lib/rate-limiter/rate-limiter'
Expand All @@ -33,11 +32,9 @@ export function createChatRoutes(deps: ChatRouteDeps) {
const executionDir = deps.executionDir || PATHS.DEFAULT_EXECUTION_DIR

const sessionManager = new SessionManager()
const klavisClient = new KlavisClient()

const chatService = new ChatService({
sessionManager,
klavisClient,
executionDir,
mcpServerUrl,
browserosId,
Expand Down
25 changes: 24 additions & 1 deletion apps/server/src/api/routes/klavis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Hono } from 'hono'
import { z } from 'zod'
import { KlavisClient } from '../../lib/clients/klavis/klavis-client'
import { OAUTH_MCP_SERVERS } from '../../lib/clients/klavis/oauth-mcp-servers'
import type { KlavisMcpProxy } from '../../lib/klavis-mcp-proxy'
import { logger } from '../../lib/logger'

const ServerNameSchema = z.object({
Expand All @@ -17,10 +18,11 @@ const ServerNameSchema = z.object({

interface KlavisRouteDeps {
browserosId: string
klavisMcpProxy?: KlavisMcpProxy
}

export function createKlavisRoutes(deps: KlavisRouteDeps) {
const { browserosId } = deps
const { browserosId, klavisMcpProxy } = deps
const klavisClient = new KlavisClient()

// Chain route definitions for proper Hono RPC type inference
Expand Down Expand Up @@ -96,6 +98,12 @@ export function createKlavisRoutes(deps: KlavisRouteDeps) {

const result = await klavisClient.createStrata(browserosId, [serverName])

klavisMcpProxy?.refresh().catch((e) => {
logger.warn('Failed to refresh Klavis MCP proxy after add', {
error: e instanceof Error ? e.message : String(e),
})
})

return c.json({
success: true,
serverName,
Expand Down Expand Up @@ -127,6 +135,15 @@ export function createKlavisRoutes(deps: KlavisRouteDeps) {

logger.info('Submitted API key for server', { serverName })

klavisMcpProxy?.refresh().catch((e) => {
logger.warn(
'Failed to refresh Klavis MCP proxy after api-key submit',
{
error: e instanceof Error ? e.message : String(e),
},
)
})

return c.json({ success: true, serverName })
} catch (error) {
logger.error('Error submitting API key', {
Expand Down Expand Up @@ -156,6 +173,12 @@ export function createKlavisRoutes(deps: KlavisRouteDeps) {

await klavisClient.removeServer(browserosId, serverName)

klavisMcpProxy?.refresh().catch((e) => {
logger.warn('Failed to refresh Klavis MCP proxy after remove', {
error: e instanceof Error ? e.message : String(e),
})
})

return c.json({
success: true,
serverName,
Expand Down
39 changes: 36 additions & 3 deletions apps/server/src/api/routes/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ import type {
} from '@modelcontextprotocol/sdk/types.js'
import { SetLevelRequestSchema } from '@modelcontextprotocol/sdk/types.js'
import { Hono } from 'hono'
import type { z } from 'zod'
import { z } from 'zod'
import type { McpContext } from '../../browser/cdp/context'
import {
type ControllerContext,
ScopedControllerContext,
} from '../../browser/extension/context'
import type { KlavisMcpProxy } from '../../lib/klavis-mcp-proxy'
import { logger } from '../../lib/logger'
import { metrics } from '../../lib/metrics'
import type { MutexPool } from '../../lib/mutex'
Expand All @@ -37,6 +38,7 @@ interface McpRouteDeps {
controllerContext: ControllerContext
mutexPool: MutexPool
allowRemote: boolean
klavisMcpProxy?: KlavisMcpProxy
}

const MCP_SOURCE_HEADER = 'X-BrowserOS-Source'
Expand All @@ -60,7 +62,14 @@ function getMcpRequestSource(
* Reuses the same logic from the old mcp/server.ts
*/
function createMcpServerWithTools(deps: McpRouteDeps): McpServer {
const { version, tools, cdpContext, controllerContext, mutexPool } = deps
const {
version,
tools,
cdpContext,
controllerContext,
mutexPool,
klavisMcpProxy,
} = deps

const server = new McpServer(
{
Expand Down Expand Up @@ -164,14 +173,38 @@ function createMcpServerWithTools(deps: McpRouteDeps): McpServer {
)
}

// Register upstream Klavis tools (proxied through Strata)
if (klavisMcpProxy?.isConnected()) {
for (const tool of klavisMcpProxy.getTools()) {
server.registerTool(
tool.name,
{
description: tool.description ?? '',
inputSchema: z.object({}).passthrough() as unknown as z.ZodRawShape,
annotations: tool.annotations,
},
async (params: Record<string, unknown>): Promise<CallToolResult> => {
return klavisMcpProxy.callTool(tool.name, params)
},
)
}
}

return server
}

export function createMcpRoutes(deps: McpRouteDeps) {
const { allowRemote } = deps

// Create MCP server once with all tools registered
const mcpServer = createMcpServerWithTools(deps)
let mcpServer = createMcpServerWithTools(deps)

if (deps.klavisMcpProxy) {
deps.klavisMcpProxy.onToolsChanged = () => {
mcpServer = createMcpServerWithTools(deps)
logger.info('MCP server rebuilt with updated Klavis tools')
}
Comment on lines +203 to +206
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

race condition: requests in-flight when tools change will use the old mcpServer instance

When onToolsChanged callback reassigns mcpServer, concurrent requests that already captured the old reference will continue using outdated tool registrations.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/api/routes/mcp.ts
Line: 203:206

Comment:
race condition: requests in-flight when tools change will use the old `mcpServer` instance

When `onToolsChanged` callback reassigns `mcpServer`, concurrent requests that already captured the old reference will continue using outdated tool registrations.

How can I resolve this? If you propose a fix, please make it concise.

}

return new Hono<Env>().all('/', async (c) => {
// Security check: localhost only (unless allowRemote is enabled)
Expand Down
7 changes: 6 additions & 1 deletion apps/server/src/api/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export async function createHttpServer(config: HttpServerConfig) {
controllerContext,
mutexPool,
allowRemote,
klavisMcpProxy,
} = config

const { onShutdown } = config
Expand All @@ -78,7 +79,10 @@ export async function createHttpServer(config: HttpServerConfig) {
)
.route('/status', createStatusRoute({ controllerContext }))
.route('/test-provider', createProviderRoutes())
.route('/klavis', createKlavisRoutes({ browserosId: browserosId || '' }))
.route(
'/klavis',
createKlavisRoutes({ browserosId: browserosId || '', klavisMcpProxy }),
)
.route(
'/mcp',
createMcpRoutes({
Expand All @@ -88,6 +92,7 @@ export async function createHttpServer(config: HttpServerConfig) {
controllerContext,
mutexPool,
allowRemote,
klavisMcpProxy,
}),
)
.route(
Expand Down
28 changes: 1 addition & 27 deletions apps/server/src/api/services/chat-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
fetchBrowserOSConfig,
getLLMConfigFromProvider,
} from '../../lib/clients/gateway'
import type { KlavisClient } from '../../lib/clients/klavis/klavis-client'
import { logger } from '../../lib/logger'
import {
detectMcpTransport,
Expand Down Expand Up @@ -48,7 +47,6 @@ function createMcpServerConfig(options: McpServerOptions): MCPServerConfig {

export interface ChatServiceDeps {
sessionManager: SessionManager
klavisClient: KlavisClient
executionDir: string
mcpServerUrl: string
browserosId?: string
Expand Down Expand Up @@ -163,7 +161,7 @@ export class ChatService {
private async buildMcpServers(
browserContext?: BrowserContext,
): Promise<Record<string, MCPServerConfig>> {
const { klavisClient, mcpServerUrl, browserosId } = this.deps
const { mcpServerUrl } = this.deps
const servers: Record<string, MCPServerConfig> = {}

if (mcpServerUrl) {
Expand All @@ -181,30 +179,6 @@ export class ChatService {
})
}

if (browserosId && browserContext?.enabledMcpServers?.length) {
try {
const result = await klavisClient.createStrata(
browserosId,
browserContext.enabledMcpServers,
)
servers['klavis-strata'] = createMcpServerConfig({
url: result.strataServerUrl,
transport: 'streamable-http',
trust: true,
})
logger.info('Added Klavis Strata MCP server', {
browserosId: browserosId.slice(0, 12),
servers: browserContext.enabledMcpServers,
})
} catch (error) {
logger.error('Failed to create Klavis Strata MCP server', {
browserosId: browserosId?.slice(0, 12),
servers: browserContext.enabledMcpServers,
error: error instanceof Error ? error.message : String(error),
})
}
}

if (browserContext?.customMcpServers?.length) {
const customServers = browserContext.customMcpServers
const transports = await Promise.all(
Expand Down
4 changes: 4 additions & 0 deletions apps/server/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { VercelAIConfigSchema } from '../agent/provider-adapter/types'
import type { McpContext } from '../browser/cdp/context'
import type { ControllerContext } from '../browser/extension/context'

import type { KlavisMcpProxy } from '../lib/klavis-mcp-proxy'
import type { MutexPool } from '../lib/mutex'
import type { RateLimiter } from '../lib/rate-limiter/rate-limiter'
import type { ToolDefinition } from '../tools/types/tool-definition'
Expand Down Expand Up @@ -74,6 +75,9 @@ export interface HttpServerConfig {
mutexPool: MutexPool
allowRemote: boolean

// For Klavis MCP proxy
klavisMcpProxy?: KlavisMcpProxy

// For Chat/Klavis routes
browserosId?: string
executionDir?: string
Expand Down
Loading
Loading