mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
Hide browser tools unless enabled
Browser tools are now hidden from agent tool catalogs unless daemon browser tools are enabled.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -110,6 +110,7 @@ export interface PaseoToolHostDependencies {
|
||||
cwd: string,
|
||||
firstAgentContext?: FirstAgentContext,
|
||||
) => Promise<string>;
|
||||
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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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<string, RegisteredBrowserHost>();
|
||||
@@ -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<BrowserToolsResponsePayload> {
|
||||
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,
|
||||
|
||||
@@ -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<DaemonConfigStore, "get">) {}
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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<BrowserToolsDaemonHarne
|
||||
|
||||
function createBroker(): BrowserToolsBroker {
|
||||
return new BrowserToolsBroker({
|
||||
policy: new StaticBrowserToolsPolicy(true),
|
||||
defaultTimeoutMs: 500,
|
||||
createRequestId: createRequestIdSequence(),
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user