Merge pull request #796 from gabriel-trigo/bugfix/ai-snapshot-limit
Fix AI snapshot size overflow
This commit is contained in:
@@ -10,6 +10,7 @@
|
|||||||
- Anthropic: merge consecutive user turns (preserve newest metadata) before validation to avoid “Incorrect role information” errors. (#804 — thanks @ThomsenDrake)
|
- 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.
|
- 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)
|
- 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)
|
- 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)
|
- 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)
|
- Messaging: enforce context isolation for message tool sends across providers (normalized targets + tests). (#793 — thanks @hsrvc)
|
||||||
|
|||||||
105
src/agents/tools/browser-tool.test.ts
Normal file
105
src/agents/tools/browser-tool.test.ts
Normal file
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -20,6 +20,7 @@ import {
|
|||||||
browserScreenshotAction,
|
browserScreenshotAction,
|
||||||
} from "../../browser/client-actions.js";
|
} from "../../browser/client-actions.js";
|
||||||
import { resolveBrowserConfig } from "../../browser/config.js";
|
import { resolveBrowserConfig } from "../../browser/config.js";
|
||||||
|
import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "../../browser/constants.js";
|
||||||
import { loadConfig } from "../../config/config.js";
|
import { loadConfig } from "../../config/config.js";
|
||||||
import {
|
import {
|
||||||
type AnyAgentTool,
|
type AnyAgentTool,
|
||||||
@@ -44,8 +45,6 @@ const BROWSER_ACT_KINDS = [
|
|||||||
|
|
||||||
type BrowserActKind = (typeof BROWSER_ACT_KINDS)[number];
|
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(...), ...])
|
// 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.
|
// because Claude API on Vertex AI rejects nested anyOf schemas as invalid JSON Schema.
|
||||||
// The discriminator (kind) determines which properties are relevant; runtime validates.
|
// 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 === "ai" || params.format === "aria"
|
||||||
? (params.format as "ai" | "aria")
|
? (params.format as "ai" | "aria")
|
||||||
: "ai";
|
: "ai";
|
||||||
|
const hasMaxChars = Object.hasOwn(params, "maxChars");
|
||||||
const targetId =
|
const targetId =
|
||||||
typeof params.targetId === "string"
|
typeof params.targetId === "string"
|
||||||
? params.targetId.trim()
|
? params.targetId.trim()
|
||||||
@@ -339,7 +339,9 @@ export function createBrowserTool(opts?: {
|
|||||||
: undefined;
|
: undefined;
|
||||||
const resolvedMaxChars =
|
const resolvedMaxChars =
|
||||||
format === "ai"
|
format === "ai"
|
||||||
? (maxChars ?? DEFAULT_AI_SNAPSHOT_MAX_CHARS)
|
? hasMaxChars
|
||||||
|
? maxChars
|
||||||
|
: DEFAULT_AI_SNAPSHOT_MAX_CHARS
|
||||||
: undefined;
|
: undefined;
|
||||||
const interactive =
|
const interactive =
|
||||||
typeof params.interactive === "boolean"
|
typeof params.interactive === "boolean"
|
||||||
@@ -361,7 +363,9 @@ export function createBrowserTool(opts?: {
|
|||||||
format,
|
format,
|
||||||
targetId,
|
targetId,
|
||||||
limit,
|
limit,
|
||||||
...(resolvedMaxChars ? { maxChars: resolvedMaxChars } : {}),
|
...(typeof resolvedMaxChars === "number"
|
||||||
|
? { maxChars: resolvedMaxChars }
|
||||||
|
: {}),
|
||||||
interactive,
|
interactive,
|
||||||
compact,
|
compact,
|
||||||
depth,
|
depth,
|
||||||
|
|||||||
@@ -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_CONTROL_URL = "http://127.0.0.1:18791";
|
||||||
export const DEFAULT_CLAWD_BROWSER_COLOR = "#FF4500";
|
export const DEFAULT_CLAWD_BROWSER_COLOR = "#FF4500";
|
||||||
export const DEFAULT_CLAWD_BROWSER_PROFILE_NAME = "clawd";
|
export const DEFAULT_CLAWD_BROWSER_PROFILE_NAME = "clawd";
|
||||||
|
export const DEFAULT_AI_SNAPSHOT_MAX_CHARS = 80_000;
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import type express from "express";
|
|||||||
import { ensureMediaDir, saveMediaBuffer } from "../../media/store.js";
|
import { ensureMediaDir, saveMediaBuffer } from "../../media/store.js";
|
||||||
import { captureScreenshot, snapshotAria } from "../cdp.js";
|
import { captureScreenshot, snapshotAria } from "../cdp.js";
|
||||||
import type { BrowserFormField } from "../client-actions-core.js";
|
import type { BrowserFormField } from "../client-actions-core.js";
|
||||||
|
import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "../constants.js";
|
||||||
import {
|
import {
|
||||||
DEFAULT_BROWSER_SCREENSHOT_MAX_BYTES,
|
DEFAULT_BROWSER_SCREENSHOT_MAX_BYTES,
|
||||||
DEFAULT_BROWSER_SCREENSHOT_MAX_SIDE,
|
DEFAULT_BROWSER_SCREENSHOT_MAX_SIDE,
|
||||||
@@ -1194,6 +1195,7 @@ export function registerBrowserAgentRoutes(
|
|||||||
: "aria";
|
: "aria";
|
||||||
const limitRaw =
|
const limitRaw =
|
||||||
typeof req.query.limit === "string" ? Number(req.query.limit) : undefined;
|
typeof req.query.limit === "string" ? Number(req.query.limit) : undefined;
|
||||||
|
const hasMaxChars = Object.hasOwn(req.query, "maxChars");
|
||||||
const maxCharsRaw =
|
const maxCharsRaw =
|
||||||
typeof req.query.maxChars === "string"
|
typeof req.query.maxChars === "string"
|
||||||
? Number(req.query.maxChars)
|
? Number(req.query.maxChars)
|
||||||
@@ -1205,6 +1207,12 @@ export function registerBrowserAgentRoutes(
|
|||||||
maxCharsRaw > 0
|
maxCharsRaw > 0
|
||||||
? Math.floor(maxCharsRaw)
|
? Math.floor(maxCharsRaw)
|
||||||
: undefined;
|
: undefined;
|
||||||
|
const resolvedMaxChars =
|
||||||
|
format === "ai"
|
||||||
|
? hasMaxChars
|
||||||
|
? maxChars
|
||||||
|
: DEFAULT_AI_SNAPSHOT_MAX_CHARS
|
||||||
|
: undefined;
|
||||||
const interactive = toBoolean(req.query.interactive);
|
const interactive = toBoolean(req.query.interactive);
|
||||||
const compact = toBoolean(req.query.compact);
|
const compact = toBoolean(req.query.compact);
|
||||||
const depth = toNumber(req.query.depth);
|
const depth = toNumber(req.query.depth);
|
||||||
@@ -1239,7 +1247,9 @@ export function registerBrowserAgentRoutes(
|
|||||||
.snapshotAiViaPlaywright({
|
.snapshotAiViaPlaywright({
|
||||||
cdpUrl: profileCtx.profile.cdpUrl,
|
cdpUrl: profileCtx.profile.cdpUrl,
|
||||||
targetId: tab.targetId,
|
targetId: tab.targetId,
|
||||||
...(maxChars ? { maxChars } : {}),
|
...(typeof resolvedMaxChars === "number"
|
||||||
|
? { maxChars: resolvedMaxChars }
|
||||||
|
: {}),
|
||||||
})
|
})
|
||||||
.catch(async (err) => {
|
.catch(async (err) => {
|
||||||
// Public-API fallback when Playwright's private _snapshotForAI is missing.
|
// Public-API fallback when Playwright's private _snapshotForAI is missing.
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { type AddressInfo, createServer } from "node:net";
|
|||||||
import { fetch as realFetch } from "undici";
|
import { fetch as realFetch } from "undici";
|
||||||
|
|
||||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "./constants.js";
|
||||||
|
|
||||||
let testPort = 0;
|
let testPort = 0;
|
||||||
let cdpBaseUrl = "";
|
let cdpBaseUrl = "";
|
||||||
@@ -329,6 +330,7 @@ describe("browser control server", () => {
|
|||||||
expect(pwMocks.snapshotAiViaPlaywright).toHaveBeenCalledWith({
|
expect(pwMocks.snapshotAiViaPlaywright).toHaveBeenCalledWith({
|
||||||
cdpUrl: cdpBaseUrl,
|
cdpUrl: cdpBaseUrl,
|
||||||
targetId: "abcd1234",
|
targetId: "abcd1234",
|
||||||
|
maxChars: DEFAULT_AI_SNAPSHOT_MAX_CHARS,
|
||||||
});
|
});
|
||||||
|
|
||||||
const nav = (await realFetch(`${base}/navigate`, {
|
const nav = (await realFetch(`${base}/navigate`, {
|
||||||
@@ -685,6 +687,25 @@ describe("browser control server", () => {
|
|||||||
expect(stopped.stopped).toBe(true);
|
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 () => {
|
it("validates agent inputs (agent routes)", async () => {
|
||||||
const { startBrowserControlServerFromConfig } = await import("./server.js");
|
const { startBrowserControlServerFromConfig } = await import("./server.js");
|
||||||
await startBrowserControlServerFromConfig();
|
await startBrowserControlServerFromConfig();
|
||||||
|
|||||||
Reference in New Issue
Block a user