plugin(diffs): optimize rendering for image/view modes

This commit is contained in:
Gustavo Madeira Santana
2026-02-28 20:19:13 -05:00
parent fcb6859784
commit 0abf47cfd5
9 changed files with 443 additions and 32 deletions

View File

@@ -0,0 +1,115 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import type { OpenClawConfig } from "openclaw/plugin-sdk";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
const { launchMock } = vi.hoisted(() => ({
launchMock: vi.fn(),
}));
vi.mock("playwright-core", () => ({
chromium: {
launch: launchMock,
},
}));
describe("PlaywrightDiffScreenshotter", () => {
let rootDir: string;
let outputPath: string;
beforeEach(async () => {
vi.useFakeTimers();
rootDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-diffs-browser-"));
outputPath = path.join(rootDir, "preview.png");
launchMock.mockReset();
const browserModule = await import("./browser.js");
await browserModule.resetSharedBrowserStateForTests();
});
afterEach(async () => {
const browserModule = await import("./browser.js");
await browserModule.resetSharedBrowserStateForTests();
vi.useRealTimers();
await fs.rm(rootDir, { recursive: true, force: true });
});
it("reuses the same browser across renders and closes it after the idle window", async () => {
const pages: Array<{ close: ReturnType<typeof vi.fn> }> = [];
const browser = createMockBrowser(pages);
launchMock.mockResolvedValue(browser);
const { PlaywrightDiffScreenshotter } = await import("./browser.js");
const screenshotter = new PlaywrightDiffScreenshotter({
config: createConfig(),
browserIdleMs: 1_000,
});
await screenshotter.screenshotHtml({
html: '<html><head></head><body><main class="oc-frame"></main></body></html>',
outputPath,
theme: "dark",
});
await screenshotter.screenshotHtml({
html: '<html><head></head><body><main class="oc-frame"></main></body></html>',
outputPath,
theme: "dark",
});
expect(launchMock).toHaveBeenCalledTimes(1);
expect(browser.newPage).toHaveBeenCalledTimes(2);
expect(pages).toHaveLength(2);
expect(pages[0]?.close).toHaveBeenCalledTimes(1);
expect(pages[1]?.close).toHaveBeenCalledTimes(1);
await vi.advanceTimersByTimeAsync(1_000);
expect(browser.close).toHaveBeenCalledTimes(1);
await screenshotter.screenshotHtml({
html: '<html><head></head><body><main class="oc-frame"></main></body></html>',
outputPath,
theme: "light",
});
expect(launchMock).toHaveBeenCalledTimes(2);
});
});
function createConfig(): OpenClawConfig {
return {
browser: {
executablePath: process.execPath,
},
} as OpenClawConfig;
}
function createMockBrowser(pages: Array<{ close: ReturnType<typeof vi.fn> }>) {
const browser = {
newPage: vi.fn(async () => {
const page = createMockPage();
pages.push(page);
return page;
}),
close: vi.fn(async () => {}),
on: vi.fn(),
};
return browser;
}
function createMockPage() {
return {
route: vi.fn(async () => {}),
setContent: vi.fn(async () => {}),
waitForFunction: vi.fn(async () => {}),
evaluate: vi.fn(async () => {}),
locator: vi.fn(() => ({
waitFor: vi.fn(async () => {}),
boundingBox: vi.fn(async () => ({ x: 40, y: 40, width: 640, height: 240 })),
})),
setViewportSize: vi.fn(async () => {}),
screenshot: vi.fn(async ({ path: screenshotPath }: { path: string }) => {
await fs.writeFile(screenshotPath, Buffer.from("png"));
}),
close: vi.fn(async () => {}),
};
}

View File

@@ -6,15 +6,43 @@ import { chromium } from "playwright-core";
import type { DiffTheme } from "./types.js";
import { VIEWER_ASSET_PREFIX, getServedViewerAsset } from "./viewer-assets.js";
const DEFAULT_BROWSER_IDLE_MS = 30_000;
const SHARED_BROWSER_KEY = "__default__";
export type DiffScreenshotter = {
screenshotHtml(params: { html: string; outputPath: string; theme: DiffTheme }): Promise<string>;
};
type BrowserInstance = Awaited<ReturnType<typeof chromium.launch>>;
type BrowserLease = {
browser: BrowserInstance;
release(): Promise<void>;
};
type SharedBrowserState = {
browser?: BrowserInstance;
browserPromise: Promise<BrowserInstance>;
idleTimer: ReturnType<typeof setTimeout> | null;
key: string;
users: number;
};
type ExecutablePathCache = {
key: string;
valuePromise: Promise<string | undefined>;
};
let sharedBrowserState: SharedBrowserState | null = null;
let executablePathCache: ExecutablePathCache | null = null;
export class PlaywrightDiffScreenshotter implements DiffScreenshotter {
private readonly config: OpenClawConfig;
private readonly browserIdleMs: number;
constructor(params: { config: OpenClawConfig }) {
constructor(params: { config: OpenClawConfig; browserIdleMs?: number }) {
this.config = params.config;
this.browserIdleMs = params.browserIdleMs ?? DEFAULT_BROWSER_IDLE_MS;
}
async screenshotHtml(params: {
@@ -23,17 +51,14 @@ export class PlaywrightDiffScreenshotter implements DiffScreenshotter {
theme: DiffTheme;
}): Promise<string> {
await fs.mkdir(path.dirname(params.outputPath), { recursive: true });
const executablePath = await resolveBrowserExecutablePath(this.config);
let browser: Awaited<ReturnType<typeof chromium.launch>> | undefined;
const lease = await acquireSharedBrowser({
config: this.config,
idleMs: this.browserIdleMs,
});
let page: Awaited<ReturnType<BrowserInstance["newPage"]>> | undefined;
try {
browser = await chromium.launch({
headless: true,
...(executablePath ? { executablePath } : {}),
args: ["--disable-dev-shm-usage", "--disable-gpu"],
});
const page = await browser.newPage({
page = await lease.browser.newPage({
viewport: { width: 1200, height: 900 },
colorScheme: params.theme,
});
@@ -113,11 +138,17 @@ export class PlaywrightDiffScreenshotter implements DiffScreenshotter {
`Diff image rendering requires a Chromium-compatible browser. Set browser.executablePath or install Chrome/Chromium. ${reason}`,
);
} finally {
await browser?.close().catch(() => {});
await page?.close().catch(() => {});
await lease.release();
}
}
}
export async function resetSharedBrowserStateForTests(): Promise<void> {
executablePathCache = null;
await closeSharedBrowser();
}
function injectBaseHref(html: string): string {
if (html.includes("<base ")) {
return html;
@@ -126,6 +157,36 @@ function injectBaseHref(html: string): string {
}
async function resolveBrowserExecutablePath(config: OpenClawConfig): Promise<string | undefined> {
const cacheKey = JSON.stringify({
configPath: config.browser?.executablePath?.trim() || "",
env: [
process.env.OPENCLAW_BROWSER_EXECUTABLE_PATH ?? "",
process.env.BROWSER_EXECUTABLE_PATH ?? "",
process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH ?? "",
],
path: process.env.PATH ?? "",
});
if (executablePathCache?.key === cacheKey) {
return await executablePathCache.valuePromise;
}
const valuePromise = resolveBrowserExecutablePathUncached(config).catch((error) => {
if (executablePathCache?.valuePromise === valuePromise) {
executablePathCache = null;
}
throw error;
});
executablePathCache = {
key: cacheKey,
valuePromise,
};
return await valuePromise;
}
async function resolveBrowserExecutablePathUncached(
config: OpenClawConfig,
): Promise<string | undefined> {
const configPath = config.browser?.executablePath?.trim();
if (configPath) {
await assertExecutable(configPath, "browser.executablePath");
@@ -155,6 +216,99 @@ async function resolveBrowserExecutablePath(config: OpenClawConfig): Promise<str
return undefined;
}
async function acquireSharedBrowser(params: {
config: OpenClawConfig;
idleMs: number;
}): Promise<BrowserLease> {
const executablePath = await resolveBrowserExecutablePath(params.config);
const desiredKey = executablePath || SHARED_BROWSER_KEY;
if (sharedBrowserState && sharedBrowserState.key !== desiredKey) {
await closeSharedBrowser();
}
if (!sharedBrowserState) {
const browserPromise = chromium
.launch({
headless: true,
...(executablePath ? { executablePath } : {}),
args: ["--disable-dev-shm-usage", "--disable-gpu"],
})
.then((browser) => {
if (sharedBrowserState?.browserPromise === browserPromise) {
sharedBrowserState.browser = browser;
browser.on("disconnected", () => {
if (sharedBrowserState?.browser === browser) {
clearIdleTimer(sharedBrowserState);
sharedBrowserState = null;
}
});
}
return browser;
})
.catch((error) => {
if (sharedBrowserState?.browserPromise === browserPromise) {
sharedBrowserState = null;
}
throw error;
});
sharedBrowserState = {
browserPromise,
idleTimer: null,
key: desiredKey,
users: 0,
};
}
clearIdleTimer(sharedBrowserState);
const state = sharedBrowserState;
const browser = await state.browserPromise;
state.users += 1;
let released = false;
return {
browser,
release: async () => {
if (released) {
return;
}
released = true;
state.users = Math.max(0, state.users - 1);
if (state.users === 0) {
scheduleIdleBrowserClose(state, params.idleMs);
}
},
};
}
function scheduleIdleBrowserClose(state: SharedBrowserState, idleMs: number): void {
clearIdleTimer(state);
state.idleTimer = setTimeout(() => {
if (sharedBrowserState === state && state.users === 0) {
void closeSharedBrowser();
}
}, idleMs);
}
function clearIdleTimer(state: SharedBrowserState): void {
if (!state.idleTimer) {
return;
}
clearTimeout(state.idleTimer);
state.idleTimer = null;
}
async function closeSharedBrowser(): Promise<void> {
const state = sharedBrowserState;
if (!state) {
return;
}
sharedBrowserState = null;
clearIdleTimer(state);
const browser = state.browser ?? (await state.browserPromise.catch(() => null));
await browser?.close().catch(() => {});
}
async function collectExecutableCandidates(): Promise<string[]> {
const candidates = new Set<string>();

View File

@@ -22,6 +22,8 @@ describe("renderDiffDocument", () => {
expect(rendered.html).toContain("data-openclaw-diff-root");
expect(rendered.html).toContain("src/example.ts");
expect(rendered.html).toContain("/plugins/diffs/assets/viewer.js");
expect(rendered.imageHtml).not.toContain("/plugins/diffs/assets/viewer.js");
expect(rendered.imageHtml).toContain('data-openclaw-diffs-ready="true"');
expect(rendered.html).not.toContain("fonts.googleapis.com");
});

View File

@@ -171,13 +171,22 @@ function renderDiffCard(payload: DiffViewerPayload): string {
</section>`;
}
function renderStaticDiffCard(prerenderedHTML: string): string {
return `<section class="oc-diff-card">
<diffs-container class="oc-diff-host" data-openclaw-diff-host>
<template shadowrootmode="open">${prerenderedHTML}</template>
</diffs-container>
</section>`;
}
function buildHtmlDocument(params: {
title: string;
bodyHtml: string;
theme: DiffRenderOptions["presentation"]["theme"];
runtimeMode: "viewer" | "image";
}): string {
return `<!doctype html>
<html lang="en">
<html lang="en"${params.runtimeMode === "image" ? ' data-openclaw-diffs-ready="true"' : ""}>
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
@@ -258,12 +267,12 @@ function buildHtmlDocument(params: {
</style>
</head>
<body data-theme="${params.theme}">
<main class="oc-frame" data-render-mode="viewer">
<main class="oc-frame" data-render-mode="${params.runtimeMode}">
<div data-openclaw-diff-root>
${params.bodyHtml}
</div>
</main>
<script type="module" src="${VIEWER_LOADER_PATH}"></script>
${params.runtimeMode === "viewer" ? `<script type="module" src="${VIEWER_LOADER_PATH}"></script>` : ""}
</body>
</html>`;
}
@@ -271,7 +280,7 @@ function buildHtmlDocument(params: {
async function renderBeforeAfterDiff(
input: Extract<DiffInput, { kind: "before_after" }>,
options: DiffRenderOptions,
): Promise<{ bodyHtml: string; fileCount: number }> {
): Promise<{ viewerBodyHtml: string; imageBodyHtml: string; fileCount: number }> {
const fileName = resolveBeforeAfterFileName(input);
const lang = normalizeSupportedLanguage(input.lang);
const oldFile: FileContents = {
@@ -292,13 +301,14 @@ async function renderBeforeAfterDiff(
});
return {
bodyHtml: renderDiffCard({
viewerBodyHtml: renderDiffCard({
prerenderedHTML: result.prerenderedHTML,
oldFile: result.oldFile,
newFile: result.newFile,
options: payloadOptions,
langs: buildPayloadLanguages({ oldFile: result.oldFile, newFile: result.newFile }),
}),
imageBodyHtml: renderStaticDiffCard(result.prerenderedHTML),
fileCount: 1,
};
}
@@ -306,7 +316,7 @@ async function renderBeforeAfterDiff(
async function renderPatchDiff(
input: Extract<DiffInput, { kind: "patch" }>,
options: DiffRenderOptions,
): Promise<{ bodyHtml: string; fileCount: number }> {
): Promise<{ viewerBodyHtml: string; imageBodyHtml: string; fileCount: number }> {
const files = parsePatchFiles(input.patch).flatMap((entry) => entry.files ?? []);
if (files.length === 0) {
throw new Error("Patch input did not contain any file diffs.");
@@ -320,17 +330,21 @@ async function renderPatchDiff(
options: payloadOptions,
});
return renderDiffCard({
prerenderedHTML: result.prerenderedHTML,
fileDiff: result.fileDiff,
options: payloadOptions,
langs: buildPayloadLanguages({ fileDiff: result.fileDiff }),
});
return {
viewer: renderDiffCard({
prerenderedHTML: result.prerenderedHTML,
fileDiff: result.fileDiff,
options: payloadOptions,
langs: buildPayloadLanguages({ fileDiff: result.fileDiff }),
}),
image: renderStaticDiffCard(result.prerenderedHTML),
};
}),
);
return {
bodyHtml: sections.join("\n"),
viewerBodyHtml: sections.map((section) => section.viewer).join("\n"),
imageBodyHtml: sections.map((section) => section.image).join("\n"),
fileCount: files.length,
};
}
@@ -348,8 +362,15 @@ export async function renderDiffDocument(
return {
html: buildHtmlDocument({
title,
bodyHtml: rendered.bodyHtml,
bodyHtml: rendered.viewerBodyHtml,
theme: options.presentation.theme,
runtimeMode: "viewer",
}),
imageHtml: buildHtmlDocument({
title,
bodyHtml: rendered.imageBodyHtml,
theme: options.presentation.theme,
runtimeMode: "image",
}),
title,
fileCount: rendered.fileCount,

View File

@@ -61,4 +61,46 @@ describe("DiffArtifactStore", () => {
const updated = await store.updateImagePath(artifact.id, imagePath);
expect(updated.imagePath).toBe(imagePath);
});
it("allocates standalone image paths outside artifact metadata", async () => {
const imagePath = store.allocateStandaloneImagePath();
expect(imagePath).toMatch(/preview\.png$/);
expect(imagePath).toContain(rootDir);
});
it("throttles cleanup sweeps across repeated artifact creation", async () => {
vi.useFakeTimers();
const now = new Date("2026-02-27T16:00:00Z");
vi.setSystemTime(now);
store = new DiffArtifactStore({
rootDir,
cleanupIntervalMs: 60_000,
});
const cleanupSpy = vi.spyOn(store, "cleanupExpired").mockResolvedValue();
await store.createArtifact({
html: "<html>one</html>",
title: "One",
inputKind: "before_after",
fileCount: 1,
});
await store.createArtifact({
html: "<html>two</html>",
title: "Two",
inputKind: "before_after",
fileCount: 1,
});
expect(cleanupSpy).toHaveBeenCalledTimes(1);
vi.setSystemTime(new Date(now.getTime() + 61_000));
await store.createArtifact({
html: "<html>three</html>",
title: "Three",
inputKind: "before_after",
fileCount: 1,
});
expect(cleanupSpy).toHaveBeenCalledTimes(2);
});
});

View File

@@ -7,6 +7,7 @@ import type { DiffArtifactMeta } from "./types.js";
const DEFAULT_TTL_MS = 30 * 60 * 1000;
const MAX_TTL_MS = 6 * 60 * 60 * 1000;
const SWEEP_FALLBACK_AGE_MS = 24 * 60 * 60 * 1000;
const DEFAULT_CLEANUP_INTERVAL_MS = 5 * 60 * 1000;
const VIEWER_PREFIX = "/plugins/diffs/view";
type CreateArtifactParams = {
@@ -20,15 +21,21 @@ type CreateArtifactParams = {
export class DiffArtifactStore {
private readonly rootDir: string;
private readonly logger?: PluginLogger;
private readonly cleanupIntervalMs: number;
private cleanupInFlight: Promise<void> | null = null;
private nextCleanupAt = 0;
constructor(params: { rootDir: string; logger?: PluginLogger }) {
constructor(params: { rootDir: string; logger?: PluginLogger; cleanupIntervalMs?: number }) {
this.rootDir = params.rootDir;
this.logger = params.logger;
this.cleanupIntervalMs =
params.cleanupIntervalMs === undefined
? DEFAULT_CLEANUP_INTERVAL_MS
: Math.max(0, Math.floor(params.cleanupIntervalMs));
}
async createArtifact(params: CreateArtifactParams): Promise<DiffArtifactMeta> {
await this.ensureRoot();
await this.cleanupExpired();
const id = crypto.randomBytes(10).toString("hex");
const token = crypto.randomBytes(24).toString("hex");
@@ -52,6 +59,7 @@ export class DiffArtifactStore {
await fs.mkdir(artifactDir, { recursive: true });
await fs.writeFile(htmlPath, params.html, "utf8");
await this.writeMeta(meta);
this.maybeCleanupExpired();
return meta;
}
@@ -95,6 +103,11 @@ export class DiffArtifactStore {
return path.join(this.artifactDir(id), "preview.png");
}
allocateStandaloneImagePath(): string {
const id = crypto.randomBytes(10).toString("hex");
return path.join(this.artifactDir(id), "preview.png");
}
async cleanupExpired(): Promise<void> {
await this.ensureRoot();
const entries = await fs.readdir(this.rootDir, { withFileTypes: true }).catch(() => []);
@@ -129,6 +142,27 @@ export class DiffArtifactStore {
await fs.mkdir(this.rootDir, { recursive: true });
}
private maybeCleanupExpired(): void {
const now = Date.now();
if (this.cleanupInFlight || now < this.nextCleanupAt) {
return;
}
this.nextCleanupAt = now + this.cleanupIntervalMs;
const cleanupPromise = this.cleanupExpired()
.catch((error) => {
this.nextCleanupAt = 0;
this.logger?.warn(`Failed to clean expired diff artifacts: ${String(error)}`);
})
.finally(() => {
if (this.cleanupInFlight === cleanupPromise) {
this.cleanupInFlight = null;
}
});
this.cleanupInFlight = cleanupPromise;
}
private artifactDir(id: string): string {
return path.join(this.rootDir, id);
}

View File

@@ -41,7 +41,8 @@ describe("diffs tool", () => {
it("returns an image artifact in image mode", async () => {
const screenshotter = {
screenshotHtml: vi.fn(async ({ outputPath }: { outputPath: string }) => {
screenshotHtml: vi.fn(async ({ html, outputPath }: { html: string; outputPath: string }) => {
expect(html).not.toContain("/plugins/diffs/assets/viewer.js");
await fs.mkdir(path.dirname(outputPath), { recursive: true });
await fs.writeFile(outputPath, Buffer.from("png"));
return outputPath;
@@ -66,6 +67,7 @@ describe("diffs tool", () => {
expect(readTextContent(result, 0)).toContain("Use the `message` tool");
expect(result?.content).toHaveLength(1);
expect((result?.details as Record<string, unknown>).imagePath).toBeDefined();
expect((result?.details as Record<string, unknown>).viewerUrl).toBeUndefined();
});
it("falls back to view output when both mode cannot render an image", async () => {
@@ -142,6 +144,14 @@ describe("diffs tool", () => {
});
it("prefers explicit tool params over configured defaults", async () => {
const screenshotter = {
screenshotHtml: vi.fn(async ({ html, outputPath }: { html: string; outputPath: string }) => {
expect(html).not.toContain("/plugins/diffs/assets/viewer.js");
await fs.mkdir(path.dirname(outputPath), { recursive: true });
await fs.writeFile(outputPath, Buffer.from("png"));
return outputPath;
}),
};
const tool = createDiffsTool({
api: createApi(),
store,
@@ -151,6 +161,7 @@ describe("diffs tool", () => {
theme: "light",
layout: "split",
},
screenshotter,
});
const result = await tool.execute?.("tool-6", {
@@ -162,6 +173,7 @@ describe("diffs tool", () => {
});
expect((result?.details as Record<string, unknown>).mode).toBe("both");
expect(screenshotter.screenshotHtml).toHaveBeenCalledTimes(1);
const viewerPath = String((result?.details as Record<string, unknown>).viewerPath);
const [id] = viewerPath.split("/").filter(Boolean).slice(-2);
const html = await store.readHtml(id);

View File

@@ -91,6 +91,39 @@ export function createDiffsTool(params: {
expandUnchanged,
});
const screenshotter =
params.screenshotter ?? new PlaywrightDiffScreenshotter({ config: params.api.config });
if (mode === "image") {
const imagePath = params.store.allocateStandaloneImagePath();
await screenshotter.screenshotHtml({
html: rendered.imageHtml,
outputPath: imagePath,
theme,
});
const imageStats = await fs.stat(imagePath);
return {
content: [
{
type: "text",
text:
`Diff image generated at: ${imagePath}\n` +
"Use the `message` tool with `path` or `filePath` to send the PNG.",
},
],
details: {
title: rendered.title,
inputKind: rendered.inputKind,
fileCount: rendered.fileCount,
mode,
imagePath,
path: imagePath,
imageBytes: imageStats.size,
},
};
}
const artifact = await params.store.createArtifact({
html: rendered.html,
title: rendered.title,
@@ -128,13 +161,10 @@ export function createDiffsTool(params: {
};
}
const screenshotter =
params.screenshotter ?? new PlaywrightDiffScreenshotter({ config: params.api.config });
try {
const imagePath = params.store.allocateImagePath(artifact.id);
await screenshotter.screenshotHtml({
html: rendered.html,
html: rendered.imageHtml,
outputPath: imagePath,
theme,
});

View File

@@ -67,6 +67,7 @@ export type DiffViewerPayload = {
export type RenderedDiffDocument = {
html: string;
imageHtml: string;
title: string;
fileCount: number;
inputKind: DiffInput["kind"];