Skip to content

Commit b5f13c5

Browse files
committed
fix: preserve remote harness output file access
1 parent da5fe17 commit b5f13c5

9 files changed

Lines changed: 287 additions & 19 deletions

File tree

packages/browseros-agent/apps/server/src/api/routes/mcp.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ import type { BrowserSession } from '../../browser/core/session'
1010
import { logger } from '../../lib/logger'
1111
import { metrics } from '../../lib/metrics'
1212
import { Sentry } from '../../lib/sentry'
13+
import {
14+
type BrowserOutputFileAccess,
15+
createBrowserOutputFileAccess,
16+
} from '../../tools/browser/output-file'
1317
import type { KlavisService } from '../services/klavis'
1418
import { createMcpServer } from '../services/mcp/mcp-server'
1519
import type { Env } from '../types'
@@ -24,6 +28,21 @@ interface McpRouteDeps {
2428
executionDir: string
2529
}
2630

31+
/** Returns generated-output readback state; scoped harnesses share it across MCP requests. */
32+
function getRemoteHarnessOutputFileAccess(
33+
accessByScope: Map<string, BrowserOutputFileAccess>,
34+
scopeId: string | undefined,
35+
): BrowserOutputFileAccess {
36+
if (!scopeId) return createBrowserOutputFileAccess()
37+
38+
const existing = accessByScope.get(scopeId)
39+
if (existing) return existing
40+
41+
const access = createBrowserOutputFileAccess()
42+
accessByScope.set(scopeId, access)
43+
return access
44+
}
45+
2746
function parseOptionalNumber(value: string | undefined): number | undefined {
2847
if (value === undefined) return undefined
2948
const n = Number(value)
@@ -57,6 +76,10 @@ export function parseManagedMcpServersHeader(
5776

5877
export function createMcpRoutes(deps: McpRouteDeps) {
5978
const app = new Hono<Env>()
79+
const remoteHarnessOutputFilesByScope = new Map<
80+
string,
81+
BrowserOutputFileAccess
82+
>()
6083

6184
app.get('/', (c) =>
6285
c.json({
@@ -66,8 +89,8 @@ export function createMcpRoutes(deps: McpRouteDeps) {
6689
)
6790

6891
app.post('/', async (c) => {
69-
const scopeId = c.req.header('X-BrowserOS-Scope-Id') || 'ephemeral'
70-
metrics.log('mcp.request', { scopeId })
92+
const scopeId = c.req.header('X-BrowserOS-Scope-Id')?.trim() || undefined
93+
metrics.log('mcp.request', { scopeId: scopeId ?? 'ephemeral' })
7194

7295
const defaultWindowId = parseOptionalNumber(
7396
c.req.header('X-BrowserOS-Default-Window-Id'),
@@ -80,6 +103,12 @@ export function createMcpRoutes(deps: McpRouteDeps) {
80103

81104
const isRemoteAgentHarness =
82105
c.req.query('source') === REMOTE_HERMES_MCP_SOURCE
106+
const outputFileAccess = isRemoteAgentHarness
107+
? getRemoteHarnessOutputFileAccess(
108+
remoteHarnessOutputFilesByScope,
109+
scopeId,
110+
)
111+
: undefined
83112

84113
// Per-request server + transport: no shared state, no race conditions,
85114
// no ID collisions. Required by MCP SDK 1.26.0+ security fix (GHSA-345p-7cg4-v4c7).
@@ -92,6 +121,7 @@ export function createMcpRoutes(deps: McpRouteDeps) {
92121
defaultTabGroupId,
93122
executionDir: deps.executionDir,
94123
isRemoteAgentHarness,
124+
outputFileAccess,
95125
})
96126
const transport = new StreamableHTTPTransport({
97127
sessionIdGenerator: undefined,
@@ -104,7 +134,7 @@ export function createMcpRoutes(deps: McpRouteDeps) {
104134
} catch (error) {
105135
Sentry.withScope((scope) => {
106136
scope.setTag('route', 'mcp')
107-
scope.setTag('scopeId', scopeId)
137+
scope.setTag('scopeId', scopeId ?? 'ephemeral')
108138
Sentry.captureException(error)
109139
})
110140
logger.error('Error handling MCP request', {

packages/browseros-agent/apps/server/src/api/services/mcp/mcp-server.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
88
import { SetLevelRequestSchema } from '@modelcontextprotocol/sdk/types.js'
99
import type { BrowserSession } from '../../../browser/core/session'
10+
import type { BrowserOutputFileAccess } from '../../../tools/browser/output-file'
1011
import type { ConnectorToolScope, KlavisService } from '../klavis'
1112
import { MCP_INSTRUCTIONS } from './mcp-prompt'
1213
import { registerTools } from './register-mcp'
@@ -20,6 +21,7 @@ export interface McpServiceDeps {
2021
defaultTabGroupId?: string
2122
executionDir: string
2223
isRemoteAgentHarness: boolean
24+
outputFileAccess?: BrowserOutputFileAccess
2325
}
2426

2527
/** Creates a per-request BrowserOS MCP server with tools for the requested surface. */
@@ -43,6 +45,7 @@ export function createMcpServer(deps: McpServiceDeps): McpServer {
4345
defaultTabGroupId: deps.defaultTabGroupId,
4446
executionDir: deps.executionDir,
4547
isRemoteAgentHarness: deps.isRemoteAgentHarness,
48+
outputFileAccess: deps.outputFileAccess,
4649
})
4750

4851
deps.klavis?.registerMcpTools(server, deps.connectorScope)

packages/browseros-agent/apps/server/src/api/services/mcp/register-mcp.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
22
import type { BrowserSession } from '../../../browser/core/session'
3+
import type { BrowserOutputFileAccess } from '../../../tools/browser/output-file'
34
import {
45
type BrowserToolDefaults,
56
registerBrowserTools,
@@ -10,6 +11,7 @@ export interface RegisterToolsDeps extends BrowserToolDefaults {
1011
browserSession: BrowserSession
1112
executionDir: string
1213
isRemoteAgentHarness: boolean
14+
outputFileAccess?: BrowserOutputFileAccess
1315
}
1416

1517
/** Registers BrowserOS MCP tools for the current request. */
@@ -22,9 +24,13 @@ export function registerTools(
2224
defaultTabGroupId: deps.defaultTabGroupId,
2325
}
2426

25-
registerBrowserTools(mcpServer, deps.browserSession, defaults)
27+
registerBrowserTools(mcpServer, deps.browserSession, defaults, {
28+
outputFileAccess: deps.outputFileAccess,
29+
})
2630

2731
if (deps.isRemoteAgentHarness) {
28-
registerFilesystemMcpTools(mcpServer, deps.executionDir)
32+
registerFilesystemMcpTools(mcpServer, deps.executionDir, {
33+
outputFileAccess: deps.outputFileAccess,
34+
})
2935
}
3036
}

packages/browseros-agent/apps/server/src/tools/browser/register.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import { logger } from '../../lib/logger'
55
import { metrics } from '../../lib/metrics'
66
import { shouldLogToolRegistration } from '../registration-log-sampling'
77
import { executeTool } from './framework'
8+
import {
9+
type BrowserOutputFileAccess,
10+
withBrowserOutputFileAccess,
11+
} from './output-file'
812
import { BROWSER_TOOLS } from './registry'
913

1014
// The SDK's registerTool is heavily overloaded/generic; retyping it to a concrete signature
@@ -31,13 +35,18 @@ export interface BrowserToolDefaults {
3135
defaultTabGroupId?: string
3236
}
3337

38+
export interface BrowserToolRegistrationOptions {
39+
outputFileAccess?: BrowserOutputFileAccess
40+
}
41+
3442
/**
3543
* Registers the browser-core tool surface on an MCP server, all bound to one BrowserSession.
3644
*/
3745
export function registerBrowserTools(
3846
server: McpServer,
3947
session: BrowserSession,
4048
defaults: BrowserToolDefaults = {},
49+
options: BrowserToolRegistrationOptions = {},
4150
): void {
4251
const register = server.registerTool.bind(server) as unknown as RegisterFn
4352

@@ -54,11 +63,15 @@ export function registerBrowserTools(
5463
async (args, extra) => {
5564
const startTime = performance.now()
5665
try {
57-
const result = await executeTool(tool, args, {
58-
session,
59-
...defaults,
60-
signal: extra?.signal,
61-
})
66+
const result = await withBrowserOutputFileAccess(
67+
options.outputFileAccess,
68+
() =>
69+
executeTool(tool, args, {
70+
session,
71+
...defaults,
72+
signal: extra?.signal,
73+
}),
74+
)
6275
metrics.log('tool_executed', {
6376
tool_name: tool.name,
6477
duration_ms: Math.round(performance.now() - startTime),

packages/browseros-agent/apps/server/src/tools/filesystem/build-toolset.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,19 @@ import { createEditTool } from './edit'
44
import { createFindTool } from './find'
55
import { createGrepTool } from './grep'
66
import { createLsTool } from './ls'
7-
import { createReadTool } from './read'
7+
import { createReadTool, type ReadToolOptions } from './read'
88
import { createWriteTool } from './write'
99

10-
export function buildFilesystemToolSet(cwd: string): ToolSet {
10+
export interface FilesystemToolSetOptions {
11+
read?: ReadToolOptions
12+
}
13+
14+
export function buildFilesystemToolSet(
15+
cwd: string,
16+
options: FilesystemToolSetOptions = {},
17+
): ToolSet {
1118
return {
12-
filesystem_read: createReadTool(cwd),
19+
filesystem_read: createReadTool(cwd, options.read),
1320
filesystem_write: createWriteTool(cwd),
1421
filesystem_edit: createEditTool(cwd),
1522
filesystem_bash: createBashTool(cwd),

packages/browseros-agent/apps/server/src/tools/filesystem/read.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const TOOL_NAME = 'filesystem_read'
2020

2121
export interface ReadToolOptions {
2222
allowedOutputPaths?: ReadonlySet<string>
23+
requireAllowedOutputPath?: boolean
2324
}
2425

2526
function createImageResult(
@@ -133,6 +134,7 @@ async function resolveReadPath(
133134
cwd: string | undefined,
134135
inputPath: string,
135136
allowedOutputPaths: ReadonlySet<string>,
137+
requireAllowedOutputPath: boolean,
136138
): Promise<string> {
137139
if (!cwd)
138140
return await resolveGeneratedOutputPath(inputPath, allowedOutputPaths)
@@ -141,6 +143,9 @@ async function resolveReadPath(
141143
return await resolveWorkspacePath(cwd, inputPath)
142144
} catch (error) {
143145
if (error instanceof Error && (await isBrowserosStatePath(inputPath))) {
146+
if (requireAllowedOutputPath) {
147+
return await resolveGeneratedOutputPath(inputPath, allowedOutputPaths)
148+
}
144149
return await resolveBrowserToolOutputPath(inputPath)
145150
}
146151
throw error
@@ -150,17 +155,18 @@ async function resolveReadPath(
150155
/** Creates the read tool for workspace files, or generated browser outputs when no workspace exists. */
151156
export function createReadTool(cwd?: string, options: ReadToolOptions = {}) {
152157
const allowedOutputPaths = options.allowedOutputPaths ?? new Set<string>()
158+
const supportsGeneratedOutputs = Boolean(options.allowedOutputPaths)
153159

154160
return tool({
155161
description: cwd
156-
? `Read a file from the filesystem. Returns text content with line numbers, or image data for image files. Text reads are limited to ${MAX_READ_LINES} lines and ${MAX_READ_CHARS} characters per call. Use offset and limit to paginate through large files.`
162+
? `Read a file from the filesystem. Returns text content with line numbers, or image data for image files. Text reads are limited to ${MAX_READ_LINES} lines and ${MAX_READ_CHARS} characters per call. Use offset and limit to paginate through large files.${supportsGeneratedOutputs ? ' Also accepts absolute BrowserOS-generated output file paths returned by browser tools.' : ''}`
157163
: `Read BrowserOS-generated tool output files by absolute path. Returns text content with line numbers, or image data for image files. Text reads are limited to ${MAX_READ_LINES} lines and ${MAX_READ_CHARS} characters per call. Use offset and limit to paginate through large files.`,
158164
inputSchema: z.object({
159165
path: z
160166
.string()
161167
.describe(
162168
cwd
163-
? 'File path relative to the selected workspace'
169+
? `File path relative to the selected workspace${supportsGeneratedOutputs ? ', or an absolute BrowserOS-generated output path returned by a browser tool' : ''}`
164170
: 'Absolute BrowserOS-generated tool output path returned by a browser tool',
165171
),
166172
offset: z
@@ -180,6 +186,7 @@ export function createReadTool(cwd?: string, options: ReadToolOptions = {}) {
180186
cwd,
181187
params.path,
182188
allowedOutputPaths,
189+
options.requireAllowedOutputPath ?? false,
183190
)
184191
const ext = extname(resolved).toLowerCase()
185192

packages/browseros-agent/apps/server/src/tools/filesystem/register-mcp.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
1414
import type { z } from 'zod'
1515
import { logger } from '../../lib/logger'
16+
import type { BrowserOutputFileAccess } from '../browser/output-file'
1617
import { shouldLogToolRegistration } from '../registration-log-sampling'
1718
import { buildFilesystemToolSet } from './build-toolset'
1819
import type { FilesystemToolResult } from './utils'
@@ -43,15 +44,22 @@ type McpRegisterFn = (
4344
}>,
4445
) => void
4546

47+
export interface RegisterFilesystemMcpToolsOptions {
48+
outputFileAccess?: BrowserOutputFileAccess
49+
}
50+
4651
export function registerFilesystemMcpTools(
4752
server: McpServer,
4853
cwd: string,
54+
options: RegisterFilesystemMcpToolsOptions = {},
4955
): void {
5056
const register = server.registerTool.bind(server) as unknown as McpRegisterFn
51-
const tools = buildFilesystemToolSet(cwd) as unknown as Record<
52-
string,
53-
AiSdkToolLike
54-
>
57+
const tools = buildFilesystemToolSet(cwd, {
58+
read: {
59+
allowedOutputPaths: options.outputFileAccess?.paths,
60+
requireAllowedOutputPath: Boolean(options.outputFileAccess),
61+
},
62+
}) as unknown as Record<string, AiSdkToolLike>
5563

5664
for (const [name, tool] of Object.entries(tools)) {
5765
register(

0 commit comments

Comments
 (0)