diff --git a/packages/server/src/server/agent/mcp-server.test.ts b/packages/server/src/server/agent/mcp-server.test.ts index ba518ac1d..bd308ac4e 100644 --- a/packages/server/src/server/agent/mcp-server.test.ts +++ b/packages/server/src/server/agent/mcp-server.test.ts @@ -766,6 +766,7 @@ describe("browser MCP tools", () => { agentStorage: agentStorage as AgentStorage, providerSnapshotManager: new BoundaryProviderSnapshotManagerFake() as unknown as ProviderSnapshotManager, + browserToolsEnabled: true, browserToolsBroker: broker as BrowserToolsBroker, callerAgentId: "agent-1", logger, @@ -839,6 +840,7 @@ describe("browser MCP tools", () => { agentStorage: agentStorage as AgentStorage, providerSnapshotManager: new BoundaryProviderSnapshotManagerFake() as unknown as ProviderSnapshotManager, + browserToolsEnabled: true, browserToolsBroker: broker as BrowserToolsBroker, callerAgentId: "agent-1", logger, @@ -893,50 +895,24 @@ describe("browser MCP tools", () => { } }); - it("keeps browser tools registered when browser tools are disabled", async () => { + it("does not register browser tools when browser tools are disabled", async () => { const { agentManager, agentStorage, spies } = createTestDeps(); spies.agentManager.getAgent.mockReturnValue({ id: "agent-1", cwd: REPO_CWD, workspaceId: BROWSER_WORKSPACE_ID, }); - const execute = vi.fn().mockResolvedValue({ - requestId: "req-browser-disabled", - ok: false, - error: { - code: "browser_disabled", - message: "Browser tools are disabled.", - retryable: false, - }, - }); const server = await createAgentMcpServer({ agentManager, agentStorage, providerSnapshotManager: createOpenCodeManager().manager, - browserToolsBroker: { execute } as never, + browserToolsEnabled: false, callerAgentId: "agent-1", logger, }); - const tool = registeredTool(server, "browser_list_tabs"); - const response = await tool.handler({}); - - expect(lookupTool(server, "browser_snapshot")).not.toBeUndefined(); - expect(execute).toHaveBeenCalledWith({ - agentId: "agent-1", - cwd: REPO_CWD, - workspaceId: BROWSER_WORKSPACE_ID, - command: { command: "list_tabs", args: {} }, - }); - expect(response.structuredContent).toEqual({ - ok: false, - error: { - code: "browser_disabled", - message: "Browser tools are disabled.", - retryable: false, - }, - context: { agentId: "agent-1", cwd: REPO_CWD, workspaceId: BROWSER_WORKSPACE_ID }, - }); + expect(lookupTool(server, "browser_list_tabs")).toBeUndefined(); + expect(lookupTool(server, "browser_snapshot")).toBeUndefined(); }); it("wires browser tools through the browser tools broker", async () => { @@ -955,6 +931,7 @@ describe("browser MCP tools", () => { agentManager, agentStorage, providerSnapshotManager: createOpenCodeManager().manager, + browserToolsEnabled: true, browserToolsBroker: { execute } as never, callerAgentId: "agent-1", logger, @@ -994,6 +971,7 @@ describe("browser MCP tools", () => { agentManager, agentStorage, providerSnapshotManager: createOpenCodeManager().manager, + browserToolsEnabled: true, browserToolsBroker: { execute } as never, callerAgentId: "agent-1", logger, diff --git a/packages/server/src/server/agent/tools/paseo-tools.ts b/packages/server/src/server/agent/tools/paseo-tools.ts index a7bf449cc..1ff0ec9ac 100644 --- a/packages/server/src/server/agent/tools/paseo-tools.ts +++ b/packages/server/src/server/agent/tools/paseo-tools.ts @@ -110,6 +110,7 @@ export interface PaseoToolHostDependencies { cwd: string, firstAgentContext?: FirstAgentContext, ) => Promise; + browserToolsEnabled?: boolean; browserToolsBroker?: BrowserToolsBroker | null; paseoHome?: string; worktreesRoot?: string; @@ -1027,7 +1028,7 @@ export function createPaseoToolCatalog(options: PaseoToolHostDependencies): Pase return toCatalog(); } - if (options.browserToolsBroker) { + if (options.browserToolsEnabled && options.browserToolsBroker) { registerBrowserTools({ registerTool, broker: options.browserToolsBroker, diff --git a/packages/server/src/server/bootstrap.ts b/packages/server/src/server/bootstrap.ts index d57d6628e..de7dbfe3b 100644 --- a/packages/server/src/server/bootstrap.ts +++ b/packages/server/src/server/bootstrap.ts @@ -484,9 +484,7 @@ export async function createPaseoDaemon( logger, ); const browserToolsPolicy = new DaemonConfigBrowserToolsPolicy(daemonConfigStore); - const browserToolsBroker = new BrowserToolsBroker({ - policy: browserToolsPolicy, - }); + const browserToolsBroker = new BrowserToolsBroker({}); const serverId = getOrCreateServerId(config.paseoHome, { logger }); const daemonKeyPair = await loadOrCreateDaemonKeyPair(config.paseoHome, logger); @@ -1018,6 +1016,7 @@ export async function createPaseoDaemon( clearWorkspaceArchiving: clearWorkspaceArchivingExternal, ensureWorkspaceForCreate: ensureWorkspaceForCreateExternal, createPaseoWorktree: createPaseoWorktreeForTools, + browserToolsEnabled: browserToolsPolicy.isEnabled(), browserToolsBroker, paseoHome: config.paseoHome, worktreesRoot: config.worktreesRoot, diff --git a/packages/server/src/server/browser-tools/broker.test.ts b/packages/server/src/server/browser-tools/broker.test.ts index b7de56e2f..aa2ffad0d 100644 --- a/packages/server/src/server/browser-tools/broker.test.ts +++ b/packages/server/src/server/browser-tools/broker.test.ts @@ -7,7 +7,6 @@ import type { } from "@getpaseo/protocol/browser-automation/rpc-schemas"; import { BROWSER_AUTOMATION_COMMAND_NAMES } from "@getpaseo/protocol/browser-automation/rpc-schemas"; import { BrowserToolsBroker, type BrowserHostClient } from "./broker.js"; -import { StaticBrowserToolsPolicy } from "./policy.js"; const BROWSER_ID = "11111111-1111-4111-8111-111111111111"; const SECOND_BROWSER_ID = "22222222-2222-4222-8222-222222222222"; @@ -65,9 +64,8 @@ class FailingBrowserHostClient implements BrowserHostClient { } } -function createBroker(options: { enabled: boolean; timeoutMs?: number }): BrowserToolsBroker { +function createBroker(options: { timeoutMs?: number } = {}): BrowserToolsBroker { return new BrowserToolsBroker({ - policy: new StaticBrowserToolsPolicy(options.enabled), defaultTimeoutMs: options.timeoutMs ?? 100, createRequestId: () => "req-1", }); @@ -82,22 +80,8 @@ describe("BrowserToolsBroker", () => { vi.useRealTimers(); }); - test("disabled returns browser_disabled", async () => { - const broker = createBroker({ enabled: false }); - - await expect(broker.execute({ command: snapshotCommand() })).resolves.toEqual({ - requestId: "req-1", - ok: false, - error: { - code: "browser_disabled", - message: "Browser tools are disabled. Enable daemon.browserTools.enabled to use them.", - retryable: false, - }, - }); - }); - test("no connected browser host returns a retryable browser_no_host error", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); await expect(broker.execute({ command: snapshotCommand() })).resolves.toEqual({ requestId: "req-1", @@ -111,7 +95,7 @@ describe("BrowserToolsBroker", () => { }); test("invalid browser requests return structured failures without contacting a host", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const client = new FakeBrowserHostClient("host-1"); broker.registerClient(client); @@ -136,7 +120,7 @@ describe("BrowserToolsBroker", () => { }); test("capable browser host receives request and returns response", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const client = new FakeBrowserHostClient("host-1"); broker.registerClient(client); @@ -194,7 +178,7 @@ describe("BrowserToolsBroker", () => { }); test("single browser host receives snapshot requests", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const client = new FakeBrowserHostClient("host-1"); broker.registerClient(client); @@ -246,7 +230,7 @@ describe("BrowserToolsBroker", () => { }); test("new tabs target the most recently registered host and tab commands stay with that host", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const firstHost = new FakeBrowserHostClient("host-1"); const recentHost = new FakeBrowserHostClient("host-2"); broker.registerClient(firstHost); @@ -319,7 +303,7 @@ describe("BrowserToolsBroker", () => { }); test("list tabs aggregates all hosts and seeds browser id affinity", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const firstHost = new FakeBrowserHostClient("host-1"); const secondHost = new FakeBrowserHostClient("host-2"); broker.registerClient(firstHost); @@ -489,7 +473,7 @@ describe("BrowserToolsBroker", () => { : never : never; }>)("routes $name to the host that owns the browser id", async ({ command, result }) => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const other = new FakeBrowserHostClient("host-1"); const owner = new FakeBrowserHostClient("host-2"); broker.registerClient(other); @@ -531,7 +515,7 @@ describe("BrowserToolsBroker", () => { }); test("successful close_tab clears browser id host affinity", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const other = new FakeBrowserHostClient("host-1"); const owner = new FakeBrowserHostClient("host-2"); broker.registerClient(other); @@ -586,7 +570,7 @@ describe("BrowserToolsBroker", () => { }); test("failed list tabs aggregation does not seed browser id affinity", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const firstHost = new FakeBrowserHostClient("host-1"); const secondHost = new FakeBrowserHostClient("host-2"); broker.registerClient(firstHost); @@ -651,7 +635,7 @@ describe("BrowserToolsBroker", () => { }); test("unsupported commands are rejected before sending to the routed host", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const client = new FakeBrowserHostClient("host-1", { supportedCommands: ["list_tabs"], hostKind: "desktop app", @@ -700,7 +684,7 @@ describe("BrowserToolsBroker", () => { }); test("unregistering a host strands its browser ids instead of routing them to another host", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const owner = new FakeBrowserHostClient("host-1"); const unregisterOwner = broker.registerClient(owner); @@ -742,7 +726,7 @@ describe("BrowserToolsBroker", () => { }); test("reconnecting the same host reclaims its stranded browser ids", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const owner = new FakeBrowserHostClient("host-1"); const unregisterOwner = broker.registerClient(owner); @@ -803,7 +787,7 @@ describe("BrowserToolsBroker", () => { test("timeout resolves browser_timeout and clears pending state", async () => { vi.useFakeTimers(); - const broker = createBroker({ enabled: true, timeoutMs: 50 }); + const broker = createBroker({ timeoutMs: 50 }); broker.registerClient(new FakeBrowserHostClient("host-1")); const resultPromise = broker.execute({ command: snapshotCommand() }); @@ -824,7 +808,7 @@ describe("BrowserToolsBroker", () => { }); test("disconnect resolves retryable failure and clears pending request", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const client = new FakeBrowserHostClient("host-1"); const unregister = broker.registerClient(client); @@ -846,7 +830,7 @@ describe("BrowserToolsBroker", () => { }); test("replacing a host registration resolves pending requests from the old registration", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const oldHost = new FakeBrowserHostClient("host-1"); const newHost = new FakeBrowserHostClient("host-1"); broker.registerClient(oldHost); @@ -884,7 +868,7 @@ describe("BrowserToolsBroker", () => { }); test("browser host send failure resolves structured failure and clears pending request", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); broker.registerClient(new FailingBrowserHostClient()); await expect(broker.execute({ command: snapshotCommand() })).resolves.toEqual({ @@ -900,7 +884,7 @@ describe("BrowserToolsBroker", () => { }); test("explicit browser failure response propagates typed error", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const client = new FakeBrowserHostClient("host-1"); broker.registerClient(client); @@ -929,7 +913,7 @@ describe("BrowserToolsBroker", () => { }); test("invalid browser response resolves a structured failure and clears pending state", async () => { - const broker = createBroker({ enabled: true }); + const broker = createBroker(); const client = new FakeBrowserHostClient("host-1"); broker.registerClient(client); diff --git a/packages/server/src/server/browser-tools/broker.ts b/packages/server/src/server/browser-tools/broker.ts index a362aad00..dc1eca7f2 100644 --- a/packages/server/src/server/browser-tools/broker.ts +++ b/packages/server/src/server/browser-tools/broker.ts @@ -8,7 +8,6 @@ import { type BrowserAutomationExecuteResponse, } from "@getpaseo/protocol/browser-automation/rpc-schemas"; import { browserToolsFailure, type BrowserToolsResponsePayload } from "./errors.js"; -import type { BrowserToolsPolicy } from "./policy.js"; export interface BrowserHostClient { id: string; @@ -40,7 +39,6 @@ interface RegisteredBrowserHost { } export interface BrowserToolsBrokerOptions { - policy: BrowserToolsPolicy; defaultTimeoutMs?: number; createRequestId?: () => string; } @@ -48,7 +46,6 @@ export interface BrowserToolsBrokerOptions { const DEFAULT_BROWSER_TOOLS_TIMEOUT_MS = 15_000; export class BrowserToolsBroker { - private readonly policy: BrowserToolsPolicy; private readonly defaultTimeoutMs: number; private readonly createRequestId: () => string; private readonly clients = new Map(); @@ -58,7 +55,6 @@ export class BrowserToolsBroker { private registrationSequence = 0; public constructor(options: BrowserToolsBrokerOptions) { - this.policy = options.policy; this.defaultTimeoutMs = options.defaultTimeoutMs ?? DEFAULT_BROWSER_TOOLS_TIMEOUT_MS; this.createRequestId = options.createRequestId ?? (() => `browser_${randomUUID()}`); } @@ -117,14 +113,6 @@ export class BrowserToolsBroker { public async execute(input: BrowserToolsExecuteInput): Promise { const requestId = input.requestId ?? this.createRequestId(); - if (!this.policy.isEnabled()) { - return browserToolsFailure({ - requestId, - code: "browser_disabled", - message: "Browser tools are disabled. Enable daemon.browserTools.enabled to use them.", - }); - } - const request = BrowserAutomationExecuteRequestSchema.safeParse({ type: "browser.automation.execute.request", requestId, diff --git a/packages/server/src/server/browser-tools/policy.ts b/packages/server/src/server/browser-tools/policy.ts index aeea65590..d91acfdc6 100644 --- a/packages/server/src/server/browser-tools/policy.ts +++ b/packages/server/src/server/browser-tools/policy.ts @@ -4,14 +4,6 @@ export interface BrowserToolsPolicy { isEnabled(): boolean; } -export class StaticBrowserToolsPolicy implements BrowserToolsPolicy { - public constructor(private readonly enabled: boolean) {} - - public isEnabled(): boolean { - return this.enabled; - } -} - export class DaemonConfigBrowserToolsPolicy implements BrowserToolsPolicy { public constructor(private readonly configStore: Pick) {} diff --git a/packages/server/src/server/browser-tools/tools.test.ts b/packages/server/src/server/browser-tools/tools.test.ts index 3ee89d967..2a1ff6dc1 100644 --- a/packages/server/src/server/browser-tools/tools.test.ts +++ b/packages/server/src/server/browser-tools/tools.test.ts @@ -472,27 +472,6 @@ const routedToolCases = [ }>; const brokerErrorCases = [ - { - name: "disabled browser tools", - toolName: "browser_list_tabs", - input: {}, - payload: { - requestId: "req-disabled", - ok: false, - error: { - code: "browser_disabled", - message: "Browser tools are disabled. Enable daemon.browserTools.enabled to use them.", - retryable: false, - }, - }, - content: [ - { - type: "text", - text: "Browser tools are disabled. Enable browser tools on the host, then try again.", - }, - ], - context: { agentId: "agent-1", cwd: "/repo", workspaceId: "wks_workspace_a" }, - }, { name: "typed timeout errors", toolName: "browser_snapshot", diff --git a/packages/server/src/server/websocket-server.browser-tools.test.ts b/packages/server/src/server/websocket-server.browser-tools.test.ts index c2cb630d9..4ee596c42 100644 --- a/packages/server/src/server/websocket-server.browser-tools.test.ts +++ b/packages/server/src/server/websocket-server.browser-tools.test.ts @@ -14,7 +14,6 @@ import { afterEach, describe, expect, it } from "vitest"; import type { AgentManager } from "./agent/agent-manager.js"; import type { AgentStorage } from "./agent/agent-storage.js"; import { BrowserToolsBroker } from "./browser-tools/broker.js"; -import { StaticBrowserToolsPolicy } from "./browser-tools/policy.js"; import type { CheckoutDiffManager } from "./checkout-diff-manager.js"; import type { FileBackedChatService } from "./chat/chat-service.js"; import type { DaemonConfigStore } from "./daemon-config-store.js"; @@ -256,7 +255,6 @@ async function startBrowserToolsDaemonHarness(): Promise