diff --git a/extensions/diffs/src/browser.test.ts b/extensions/diffs/src/browser.test.ts index 56251aad236..1498561cfa3 100644 --- a/extensions/diffs/src/browser.test.ts +++ b/extensions/diffs/src/browser.test.ts @@ -35,19 +35,7 @@ describe("PlaywrightDiffScreenshotter", () => { }); it("reuses the same browser across renders and closes it after the idle window", async () => { - const pages: Array<{ - close: ReturnType; - screenshot: ReturnType; - pdf: ReturnType; - }> = []; - const browser = createMockBrowser(pages); - launchMock.mockResolvedValue(browser); - const { PlaywrightDiffScreenshotter } = await import("./browser.js"); - - const screenshotter = new PlaywrightDiffScreenshotter({ - config: createConfig(), - browserIdleMs: 1_000, - }); + const { pages, browser, screenshotter } = await createScreenshotterHarness(); await screenshotter.screenshotHtml({ html: '
', @@ -106,19 +94,7 @@ describe("PlaywrightDiffScreenshotter", () => { }); it("renders PDF output when format is pdf", async () => { - const pages: Array<{ - close: ReturnType; - screenshot: ReturnType; - pdf: ReturnType; - }> = []; - const browser = createMockBrowser(pages); - launchMock.mockResolvedValue(browser); - const { PlaywrightDiffScreenshotter } = await import("./browser.js"); - - const screenshotter = new PlaywrightDiffScreenshotter({ - config: createConfig(), - browserIdleMs: 1_000, - }); + const { pages, browser, screenshotter } = await createScreenshotterHarness(); const pdfPath = path.join(rootDir, "preview.pdf"); await screenshotter.screenshotHtml({ @@ -184,19 +160,7 @@ describe("PlaywrightDiffScreenshotter", () => { }); it("fails fast when maxPixels is still exceeded at scale 1", async () => { - const pages: Array<{ - close: ReturnType; - screenshot: ReturnType; - pdf: ReturnType; - }> = []; - const browser = createMockBrowser(pages); - launchMock.mockResolvedValue(browser); - const { PlaywrightDiffScreenshotter } = await import("./browser.js"); - - const screenshotter = new PlaywrightDiffScreenshotter({ - config: createConfig(), - browserIdleMs: 1_000, - }); + const { pages, screenshotter } = await createScreenshotterHarness(); await expect( screenshotter.screenshotHtml({ @@ -225,6 +189,24 @@ function createConfig(): OpenClawConfig { } as OpenClawConfig; } +async function createScreenshotterHarness(options?: { + boundingBox?: { x: number; y: number; width: number; height: number }; +}) { + const pages: Array<{ + close: ReturnType; + screenshot: ReturnType; + pdf: ReturnType; + }> = []; + const browser = createMockBrowser(pages, options); + launchMock.mockResolvedValue(browser); + const { PlaywrightDiffScreenshotter } = await import("./browser.js"); + const screenshotter = new PlaywrightDiffScreenshotter({ + config: createConfig(), + browserIdleMs: 1_000, + }); + return { pages, browser, screenshotter }; +} + function createMockBrowser( pages: Array<{ close: ReturnType; diff --git a/extensions/diffs/src/tool.test.ts b/extensions/diffs/src/tool.test.ts index 23b71b1e6eb..bef38ca9699 100644 --- a/extensions/diffs/src/tool.test.ts +++ b/extensions/diffs/src/tool.test.ts @@ -3,9 +3,11 @@ import os from "node:os"; import path from "node:path"; import type { OpenClawPluginApi } from "openclaw/plugin-sdk"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { DiffScreenshotter } from "./browser.js"; import { DEFAULT_DIFFS_TOOL_DEFAULTS } from "./config.js"; import { DiffArtifactStore } from "./store.js"; import { createDiffsTool } from "./tool.js"; +import type { DiffRenderOptions } from "./types.js"; describe("diffs tool", () => { let rootDir: string; @@ -53,38 +55,22 @@ describe("diffs tool", () => { it("returns an image artifact in image mode", async () => { const cleanupSpy = vi.spyOn(store, "scheduleCleanup"); - const screenshotter = { - screenshotHtml: vi.fn( - async ({ - html, - outputPath, - image, - }: { - html: string; - outputPath: string; - image: { format: string; qualityPreset: string; scale: number; maxWidth: number }; - }) => { - expect(html).not.toContain("/plugins/diffs/assets/viewer.js"); - expect(image).toMatchObject({ - format: "png", - qualityPreset: "standard", - scale: 2, - maxWidth: 960, - }); - await fs.mkdir(path.dirname(outputPath), { recursive: true }); - await fs.writeFile(outputPath, Buffer.from("png")); - return outputPath; - }, - ), - }; - - const tool = createDiffsTool({ - api: createApi(), - store, - defaults: DEFAULT_DIFFS_TOOL_DEFAULTS, - screenshotter, + const screenshotter = createPngScreenshotter({ + assertHtml: (html) => { + expect(html).not.toContain("/plugins/diffs/assets/viewer.js"); + }, + assertImage: (image) => { + expect(image).toMatchObject({ + format: "png", + qualityPreset: "standard", + scale: 2, + maxWidth: 960, + }); + }, }); + const tool = createToolWithScreenshotter(store, screenshotter); + const result = await tool.execute?.("tool-2", { before: "one\n", after: "two\n", @@ -148,22 +134,14 @@ describe("diffs tool", () => { }); it("accepts mode=file as an alias for file artifact rendering", async () => { - const screenshotter = { - screenshotHtml: vi.fn(async ({ outputPath }: { outputPath: string }) => { + const screenshotter = createPngScreenshotter({ + assertOutputPath: (outputPath) => { expect(outputPath).toMatch(/preview\.png$/); - await fs.mkdir(path.dirname(outputPath), { recursive: true }); - await fs.writeFile(outputPath, Buffer.from("png")); - return outputPath; - }), - }; - - const tool = createDiffsTool({ - api: createApi(), - store, - defaults: DEFAULT_DIFFS_TOOL_DEFAULTS, - screenshotter, + }, }); + const tool = createToolWithScreenshotter(store, screenshotter); + const result = await tool.execute?.("tool-2c", { before: "one\n", after: "two\n", @@ -180,20 +158,8 @@ describe("diffs tool", () => { const now = new Date("2026-02-27T16:00:00Z"); vi.setSystemTime(now); try { - const screenshotter = { - screenshotHtml: vi.fn(async ({ outputPath }: { outputPath: string }) => { - await fs.mkdir(path.dirname(outputPath), { recursive: true }); - await fs.writeFile(outputPath, Buffer.from("png")); - return outputPath; - }), - }; - - const tool = createDiffsTool({ - api: createApi(), - store, - defaults: DEFAULT_DIFFS_TOOL_DEFAULTS, - screenshotter, - }); + const screenshotter = createPngScreenshotter(); + const tool = createToolWithScreenshotter(store, screenshotter); const result = await tool.execute?.("tool-2c-ttl", { before: "one\n", @@ -215,34 +181,18 @@ describe("diffs tool", () => { }); it("accepts image* tool options for backward compatibility", async () => { - const screenshotter = { - screenshotHtml: vi.fn( - async ({ - outputPath, - image, - }: { - outputPath: string; - image: { qualityPreset: string; scale: number; maxWidth: number }; - }) => { - expect(image).toMatchObject({ - qualityPreset: "hq", - scale: 2.4, - maxWidth: 1100, - }); - await fs.mkdir(path.dirname(outputPath), { recursive: true }); - await fs.writeFile(outputPath, Buffer.from("png")); - return outputPath; - }, - ), - }; - - const tool = createDiffsTool({ - api: createApi(), - store, - defaults: DEFAULT_DIFFS_TOOL_DEFAULTS, - screenshotter, + const screenshotter = createPngScreenshotter({ + assertImage: (image) => { + expect(image).toMatchObject({ + qualityPreset: "hq", + scale: 2.4, + maxWidth: 1100, + }); + }, }); + const tool = createToolWithScreenshotter(store, screenshotter); + const result = await tool.execute?.("tool-2legacy", { before: "one\n", after: "two\n", @@ -294,22 +244,10 @@ describe("diffs tool", () => { }); it("honors defaults.mode=file when mode is omitted", async () => { - const screenshotter = { - screenshotHtml: vi.fn(async ({ outputPath }: { outputPath: string }) => { - await fs.mkdir(path.dirname(outputPath), { recursive: true }); - await fs.writeFile(outputPath, Buffer.from("png")); - return outputPath; - }), - }; - - const tool = createDiffsTool({ - api: createApi(), - store, - defaults: { - ...DEFAULT_DIFFS_TOOL_DEFAULTS, - mode: "file", - }, - screenshotter, + const screenshotter = createPngScreenshotter(); + const tool = createToolWithScreenshotter(store, screenshotter, { + ...DEFAULT_DIFFS_TOOL_DEFAULTS, + mode: "file", }); const result = await tool.execute?.("tool-2d", { @@ -429,43 +367,27 @@ describe("diffs tool", () => { }); it("prefers explicit tool params over configured defaults", async () => { - const screenshotter = { - screenshotHtml: vi.fn( - async ({ - html, - outputPath, - image, - }: { - html: string; - outputPath: string; - image: { format: string; qualityPreset: string; scale: number; maxWidth: number }; - }) => { - expect(html).not.toContain("/plugins/diffs/assets/viewer.js"); - expect(image).toMatchObject({ - format: "png", - qualityPreset: "print", - scale: 2.75, - maxWidth: 1320, - }); - await fs.mkdir(path.dirname(outputPath), { recursive: true }); - await fs.writeFile(outputPath, Buffer.from("png")); - return outputPath; - }, - ), - }; - const tool = createDiffsTool({ - api: createApi(), - store, - defaults: { - ...DEFAULT_DIFFS_TOOL_DEFAULTS, - mode: "view", - theme: "light", - layout: "split", - fileQuality: "hq", - fileScale: 2.2, - fileMaxWidth: 1180, + const screenshotter = createPngScreenshotter({ + assertHtml: (html) => { + expect(html).not.toContain("/plugins/diffs/assets/viewer.js"); }, - screenshotter, + assertImage: (image) => { + expect(image).toMatchObject({ + format: "png", + qualityPreset: "print", + scale: 2.75, + maxWidth: 1320, + }); + }, + }); + const tool = createToolWithScreenshotter(store, screenshotter, { + ...DEFAULT_DIFFS_TOOL_DEFAULTS, + mode: "view", + theme: "light", + layout: "split", + fileQuality: "hq", + fileScale: 2.2, + fileMaxWidth: 1180, }); const result = await tool.execute?.("tool-6", { @@ -527,6 +449,49 @@ function createApi(): OpenClawPluginApi { }; } +function createToolWithScreenshotter( + store: DiffArtifactStore, + screenshotter: DiffScreenshotter, + defaults = DEFAULT_DIFFS_TOOL_DEFAULTS, +) { + return createDiffsTool({ + api: createApi(), + store, + defaults, + screenshotter, + }); +} + +function createPngScreenshotter( + params: { + assertHtml?: (html: string) => void; + assertImage?: (image: DiffRenderOptions["image"]) => void; + assertOutputPath?: (outputPath: string) => void; + } = {}, +): DiffScreenshotter { + const screenshotHtml: DiffScreenshotter["screenshotHtml"] = vi.fn( + async ({ + html, + outputPath, + image, + }: { + html: string; + outputPath: string; + image: DiffRenderOptions["image"]; + }) => { + params.assertHtml?.(html); + params.assertImage?.(image); + params.assertOutputPath?.(outputPath); + await fs.mkdir(path.dirname(outputPath), { recursive: true }); + await fs.writeFile(outputPath, Buffer.from("png")); + return outputPath; + }, + ); + return { + screenshotHtml, + }; +} + function readTextContent(result: unknown, index: number): string { const content = (result as { content?: Array<{ type?: string; text?: string }> } | undefined) ?.content; diff --git a/extensions/diffs/src/tool.ts b/extensions/diffs/src/tool.ts index 92f9f5c778d..862fd2b0d56 100644 --- a/extensions/diffs/src/tool.ts +++ b/extensions/diffs/src/tool.ts @@ -192,25 +192,16 @@ export function createDiffsTool(params: { "Use the `message` tool with `path` or `filePath` to send this file.", }, ], - details: { - title: rendered.title, - inputKind: rendered.inputKind, - fileCount: rendered.fileCount, - mode, - filePath: artifactFile.path, - imagePath: artifactFile.path, - path: artifactFile.path, - fileBytes: artifactFile.bytes, - imageBytes: artifactFile.bytes, - format: image.format, - fileFormat: image.format, - fileQuality: image.qualityPreset, - imageQuality: image.qualityPreset, - fileScale: image.scale, - imageScale: image.scale, - fileMaxWidth: image.maxWidth, - imageMaxWidth: image.maxWidth, - }, + details: buildArtifactDetails({ + baseDetails: { + title: rendered.title, + inputKind: rendered.inputKind, + fileCount: rendered.fileCount, + mode, + }, + artifactFile, + image, + }), }; } @@ -272,22 +263,11 @@ export function createDiffsTool(params: { "Use the `message` tool with `path` or `filePath` to send this file.", }, ], - details: { - ...baseDetails, - filePath: artifactFile.path, - imagePath: artifactFile.path, - path: artifactFile.path, - fileBytes: artifactFile.bytes, - imageBytes: artifactFile.bytes, - format: image.format, - fileFormat: image.format, - fileQuality: image.qualityPreset, - imageQuality: image.qualityPreset, - fileScale: image.scale, - imageScale: image.scale, - fileMaxWidth: image.maxWidth, - imageMaxWidth: image.maxWidth, - }, + details: buildArtifactDetails({ + baseDetails, + artifactFile, + image, + }), }; } catch (error) { if (mode === "both") { @@ -327,6 +307,29 @@ function isArtifactOnlyMode(mode: DiffMode): mode is "image" | "file" { return mode === "image" || mode === "file"; } +function buildArtifactDetails(params: { + baseDetails: Record; + artifactFile: { path: string; bytes: number }; + image: DiffRenderOptions["image"]; +}) { + return { + ...params.baseDetails, + filePath: params.artifactFile.path, + imagePath: params.artifactFile.path, + path: params.artifactFile.path, + fileBytes: params.artifactFile.bytes, + imageBytes: params.artifactFile.bytes, + format: params.image.format, + fileFormat: params.image.format, + fileQuality: params.image.qualityPreset, + imageQuality: params.image.qualityPreset, + fileScale: params.image.scale, + imageScale: params.image.scale, + fileMaxWidth: params.image.maxWidth, + imageMaxWidth: params.image.maxWidth, + }; +} + async function renderDiffArtifactFile(params: { screenshotter: DiffScreenshotter; store: DiffArtifactStore;