diff --git a/CHANGELOG.md b/CHANGELOG.md index dad7a883c..c43877964 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Anthropic: merge consecutive user turns (preserve newest metadata) before validation to avoid “Incorrect role information” errors. (#804 — thanks @ThomsenDrake) - Discord/Slack: centralize reply-thread planning so auto-thread replies stay in the created thread without parent reply refs. - Update: run `clawdbot doctor --non-interactive` during updates to avoid TTY hangs. (#781 — thanks @ronyrus) +- Browser tools: treat explicit `maxChars: 0` as unlimited while keeping the default limit only when omitted. (#796 — thanks @gabriel-trigo) - Tools: allow Claude/Gemini tool param aliases (`file_path`, `old_string`, `new_string`) while enforcing required params at runtime. (#793 — thanks @hsrvc) - Gemini: downgrade tool-call history missing `thought_signature` to avoid INVALID_ARGUMENT errors. (#793 — thanks @hsrvc) - Messaging: enforce context isolation for message tool sends across providers (normalized targets + tests). (#793 — thanks @hsrvc) diff --git a/src/agents/tools/browser-tool.test.ts b/src/agents/tools/browser-tool.test.ts new file mode 100644 index 000000000..dc3439bfc --- /dev/null +++ b/src/agents/tools/browser-tool.test.ts @@ -0,0 +1,105 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +const browserClientMocks = vi.hoisted(() => ({ + browserCloseTab: vi.fn(async () => ({})), + browserFocusTab: vi.fn(async () => ({})), + browserOpenTab: vi.fn(async () => ({})), + browserSnapshot: vi.fn(async () => ({ + ok: true, + format: "ai", + targetId: "t1", + url: "https://example.com", + snapshot: "ok", + })), + browserStart: vi.fn(async () => ({})), + browserStatus: vi.fn(async () => ({ + ok: true, + running: true, + pid: 1, + cdpPort: 18792, + cdpUrl: "http://127.0.0.1:18792", + })), + browserStop: vi.fn(async () => ({})), + browserTabs: vi.fn(async () => []), +})); +vi.mock("../../browser/client.js", () => browserClientMocks); + +const browserConfigMocks = vi.hoisted(() => ({ + resolveBrowserConfig: vi.fn(() => ({ + enabled: true, + controlUrl: "http://127.0.0.1:18791", + controlHost: "127.0.0.1", + controlPort: 18791, + cdpProtocol: "http", + cdpHost: "127.0.0.1", + cdpIsLoopback: true, + color: "#FF0000", + headless: true, + noSandbox: false, + attachOnly: false, + defaultProfile: "clawd", + profiles: { + clawd: { + cdpPort: 18792, + color: "#FF0000", + }, + }, + })), +})); +vi.mock("../../browser/config.js", () => browserConfigMocks); + +vi.mock("../../config/config.js", () => ({ + loadConfig: vi.fn(() => ({ browser: {} })), +})); + +import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "../../browser/constants.js"; +import { createBrowserTool } from "./browser-tool.js"; + +describe("browser tool snapshot maxChars", () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it("applies the default ai snapshot limit", async () => { + const tool = createBrowserTool(); + await tool.execute?.(null, { action: "snapshot", format: "ai" }); + + expect(browserClientMocks.browserSnapshot).toHaveBeenCalledWith( + "http://127.0.0.1:18791", + expect.objectContaining({ + format: "ai", + maxChars: DEFAULT_AI_SNAPSHOT_MAX_CHARS, + }), + ); + }); + + it("respects an explicit maxChars override", async () => { + const tool = createBrowserTool(); + const override = 2_000; + await tool.execute?.(null, { + action: "snapshot", + format: "ai", + maxChars: override, + }); + + expect(browserClientMocks.browserSnapshot).toHaveBeenCalledWith( + "http://127.0.0.1:18791", + expect.objectContaining({ + maxChars: override, + }), + ); + }); + + it("skips the default when maxChars is explicitly zero", async () => { + const tool = createBrowserTool(); + await tool.execute?.(null, { + action: "snapshot", + format: "ai", + maxChars: 0, + }); + + expect(browserClientMocks.browserSnapshot).toHaveBeenCalled(); + const [, opts] = browserClientMocks.browserSnapshot.mock.calls.at(-1) ?? []; + expect(Object.hasOwn(opts ?? {}, "maxChars")).toBe(false); + }); +}); diff --git a/src/agents/tools/browser-tool.ts b/src/agents/tools/browser-tool.ts index 3b33bbac4..0f0bcb433 100644 --- a/src/agents/tools/browser-tool.ts +++ b/src/agents/tools/browser-tool.ts @@ -20,6 +20,7 @@ import { browserScreenshotAction, } from "../../browser/client-actions.js"; import { resolveBrowserConfig } from "../../browser/config.js"; +import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "../../browser/constants.js"; import { loadConfig } from "../../config/config.js"; import { type AnyAgentTool, @@ -44,8 +45,6 @@ const BROWSER_ACT_KINDS = [ type BrowserActKind = (typeof BROWSER_ACT_KINDS)[number]; -const DEFAULT_AI_SNAPSHOT_MAX_CHARS = 80_000; - // NOTE: Using a flattened object schema instead of Type.Union([Type.Object(...), ...]) // because Claude API on Vertex AI rejects nested anyOf schemas as invalid JSON Schema. // The discriminator (kind) determines which properties are relevant; runtime validates. @@ -323,6 +322,7 @@ export function createBrowserTool(opts?: { params.format === "ai" || params.format === "aria" ? (params.format as "ai" | "aria") : "ai"; + const hasMaxChars = Object.hasOwn(params, "maxChars"); const targetId = typeof params.targetId === "string" ? params.targetId.trim() @@ -339,7 +339,9 @@ export function createBrowserTool(opts?: { : undefined; const resolvedMaxChars = format === "ai" - ? (maxChars ?? DEFAULT_AI_SNAPSHOT_MAX_CHARS) + ? hasMaxChars + ? maxChars + : DEFAULT_AI_SNAPSHOT_MAX_CHARS : undefined; const interactive = typeof params.interactive === "boolean" @@ -361,7 +363,9 @@ export function createBrowserTool(opts?: { format, targetId, limit, - ...(resolvedMaxChars ? { maxChars: resolvedMaxChars } : {}), + ...(typeof resolvedMaxChars === "number" + ? { maxChars: resolvedMaxChars } + : {}), interactive, compact, depth, diff --git a/src/browser/constants.ts b/src/browser/constants.ts index 041a3e6ce..a5c3052d1 100644 --- a/src/browser/constants.ts +++ b/src/browser/constants.ts @@ -2,3 +2,4 @@ export const DEFAULT_CLAWD_BROWSER_ENABLED = true; export const DEFAULT_CLAWD_BROWSER_CONTROL_URL = "http://127.0.0.1:18791"; export const DEFAULT_CLAWD_BROWSER_COLOR = "#FF4500"; export const DEFAULT_CLAWD_BROWSER_PROFILE_NAME = "clawd"; +export const DEFAULT_AI_SNAPSHOT_MAX_CHARS = 80_000; diff --git a/src/browser/routes/agent.ts b/src/browser/routes/agent.ts index 91b4d5c42..fd960ca99 100644 --- a/src/browser/routes/agent.ts +++ b/src/browser/routes/agent.ts @@ -7,6 +7,7 @@ import type express from "express"; import { ensureMediaDir, saveMediaBuffer } from "../../media/store.js"; import { captureScreenshot, snapshotAria } from "../cdp.js"; import type { BrowserFormField } from "../client-actions-core.js"; +import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "../constants.js"; import { DEFAULT_BROWSER_SCREENSHOT_MAX_BYTES, DEFAULT_BROWSER_SCREENSHOT_MAX_SIDE, @@ -1194,6 +1195,7 @@ export function registerBrowserAgentRoutes( : "aria"; const limitRaw = typeof req.query.limit === "string" ? Number(req.query.limit) : undefined; + const hasMaxChars = Object.hasOwn(req.query, "maxChars"); const maxCharsRaw = typeof req.query.maxChars === "string" ? Number(req.query.maxChars) @@ -1205,6 +1207,12 @@ export function registerBrowserAgentRoutes( maxCharsRaw > 0 ? Math.floor(maxCharsRaw) : undefined; + const resolvedMaxChars = + format === "ai" + ? hasMaxChars + ? maxChars + : DEFAULT_AI_SNAPSHOT_MAX_CHARS + : undefined; const interactive = toBoolean(req.query.interactive); const compact = toBoolean(req.query.compact); const depth = toNumber(req.query.depth); @@ -1239,7 +1247,9 @@ export function registerBrowserAgentRoutes( .snapshotAiViaPlaywright({ cdpUrl: profileCtx.profile.cdpUrl, targetId: tab.targetId, - ...(maxChars ? { maxChars } : {}), + ...(typeof resolvedMaxChars === "number" + ? { maxChars: resolvedMaxChars } + : {}), }) .catch(async (err) => { // Public-API fallback when Playwright's private _snapshotForAI is missing. diff --git a/src/browser/server.test.ts b/src/browser/server.test.ts index 47e644b1e..9488b4331 100644 --- a/src/browser/server.test.ts +++ b/src/browser/server.test.ts @@ -2,6 +2,7 @@ import { type AddressInfo, createServer } from "node:net"; import { fetch as realFetch } from "undici"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "./constants.js"; let testPort = 0; let cdpBaseUrl = ""; @@ -329,6 +330,7 @@ describe("browser control server", () => { expect(pwMocks.snapshotAiViaPlaywright).toHaveBeenCalledWith({ cdpUrl: cdpBaseUrl, targetId: "abcd1234", + maxChars: DEFAULT_AI_SNAPSHOT_MAX_CHARS, }); const nav = (await realFetch(`${base}/navigate`, { @@ -685,6 +687,25 @@ describe("browser control server", () => { expect(stopped.stopped).toBe(true); }); + it("skips default maxChars when explicitly set to zero", async () => { + const { startBrowserControlServerFromConfig } = await import("./server.js"); + await startBrowserControlServerFromConfig(); + const base = `http://127.0.0.1:${testPort}`; + await realFetch(`${base}/start`, { method: "POST" }).then((r) => r.json()); + + const snapAi = (await realFetch( + `${base}/snapshot?format=ai&maxChars=0`, + ).then((r) => r.json())) as { ok: boolean; format?: string }; + expect(snapAi.ok).toBe(true); + expect(snapAi.format).toBe("ai"); + + const [call] = pwMocks.snapshotAiViaPlaywright.mock.calls.at(-1) ?? []; + expect(call).toEqual({ + cdpUrl: cdpBaseUrl, + targetId: "abcd1234", + }); + }); + it("validates agent inputs (agent routes)", async () => { const { startBrowserControlServerFromConfig } = await import("./server.js"); await startBrowserControlServerFromConfig();