fix(diffs): share SSR preloads and repair language-pack hydration (#100487)

* fix(diffs): share SSR preloads and repair language-pack hydration

Render viewer and file documents from a single @pierre/diffs SSR preload
per file (mode=both previously ran the full diff+highlight pipeline
twice; 651ms -> 303ms on an 8-file patch), apply the file-mode font bump
as a document-level override, and keep hydration payloads
variant-faithful.

Fix the language-pack runtime downgrading pack-only languages to plain
text at hydration by defining a per-target build flag and forwarding it
to payload normalization.

Also: case-insensitive language hints, identical before/after
short-circuit with details.changed, patch input failures classified as
tool input errors, canonical config values now win over deprecated
aliases, hash-pinned viewer runtime served immutable, truthful
browser-vs-render errors, timing-safe artifact token compare, unref
idle browser timer.

* docs(changelog): link diffs rendering entry to PR

* test(diffs): narrow manifest validation results before value access

* test(tooling): allowlist diffs viewer-client define suppression
This commit is contained in:
Peter Steinberger
2026-07-06 00:34:17 +01:00
committed by GitHub
parent f20159e240
commit 2ded26a5d6
25 changed files with 470 additions and 171 deletions
+1
View File
@@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- **Diffs rendering:** render viewer and image output from one SSR preload, preserve language-pack highlighting through hydration, normalize language hints case-insensitively, skip identical before/after inputs with an explicit `changed` result, report truthful file-render and input errors, cache hash-pinned viewer runtimes, and prefer canonical file settings over stale aliases. (#100487)
- **Remote browser reliability:** bound persistent Playwright tab enumeration by the existing remote CDP timeout budget and retire timed-out connection attempts so late completions cannot restore a stuck connection. (#80147, #58968) Thanks @HemantSudarshan and @KeaneYan.
- **Control UI approval prompts:** keep stale resolve failures and busy-state cleanup from leaking across newer approvals or Gateway reconnects. (#98394) Thanks @haruaiclone-droid.
- **Agent empty replies:** surface a visible failure when a completed interactive turn has no deliverable reply, including queued follow-ups, while preserving explicit silence, pending continuations, and committed side effects. (#100456) Thanks @mushuiyu886.
+4
View File
@@ -171,8 +171,11 @@ Without the pack, unsupported languages still render as readable plain text. See
## Output details contract
All successful results include `changed`: identical before/after input returns `false` without creating an artifact; rendered results return `true`.
<AccordionGroup>
<Accordion title="Viewer fields (view and both modes)">
- `changed`
- `artifactId`
- `viewerUrl`
- `viewerPath`
@@ -185,6 +188,7 @@ Without the pack, unsupported languages still render as readable plain text. See
</Accordion>
<Accordion title="File fields (file and both modes)">
- `changed`
- `artifactId`
- `expiresAt`
- `filePath`
+14 -4
View File
@@ -1,7 +1,9 @@
// Diffs Language Pack plugin module implements plugin behavior.
import type { IncomingMessage, ServerResponse } from "node:http";
import type { OpenClawPluginApi } from "../api.js";
import { VIEWER_ASSET_PREFIX, getServedViewerAsset } from "./viewer-assets.js";
import { VIEWER_ASSET_PREFIX, VIEWER_RUNTIME_PATH, getServedViewerAsset } from "./viewer-assets.js";
const IMMUTABLE_ASSET_CACHE_CONTROL = "public, max-age=31536000, immutable";
export function registerDiffsLanguagePackPlugin(api: OpenClawPluginApi): void {
api.registerHttpRoute({
@@ -30,7 +32,11 @@ function createDiffsLanguagePackHttpHandler() {
}
res.statusCode = 200;
setSharedHeaders(res, asset.contentType);
setSharedHeaders(
res,
asset.contentType,
parsed.pathname === VIEWER_RUNTIME_PATH ? IMMUTABLE_ASSET_CACHE_CONTROL : undefined,
);
if (req.method === "HEAD") {
res.end();
} else {
@@ -57,8 +63,12 @@ function respondText(res: ServerResponse, statusCode: number, body: string): voi
res.end(body);
}
function setSharedHeaders(res: ServerResponse, contentType: string): void {
res.setHeader("cache-control", "no-store, max-age=0");
function setSharedHeaders(
res: ServerResponse,
contentType: string,
cacheControl = "no-store, max-age=0",
): void {
res.setHeader("cache-control", cacheControl);
res.setHeader("content-type", contentType);
res.setHeader("x-content-type-options", "nosniff");
res.setHeader("referrer-policy", "no-referrer");
+1
View File
@@ -20,6 +20,7 @@ It gives agents one tool, `diffs`, that can:
The tool can return:
- `details.changed`: `false` when before/after inputs are identical and no artifact was rendered; `true` for rendered results
- `details.viewerUrl`: a gateway URL that can be opened in the canvas
- `details.filePath`: a local rendered artifact path when file rendering is requested
- `details.fileFormat`: the rendered file format (`png` or `pdf`)
+2 -4
View File
@@ -145,8 +145,7 @@
},
"fileFormat": {
"type": "string",
"enum": ["png", "pdf"],
"default": "png"
"enum": ["png", "pdf"]
},
"format": {
"type": "string",
@@ -155,8 +154,7 @@
},
"fileQuality": {
"type": "string",
"enum": ["standard", "hq", "print"],
"default": "standard"
"enum": ["standard", "hq", "print"]
},
"fileScale": {
"type": "number",
+2
View File
@@ -7,6 +7,8 @@ When you need to show edits as a real diff, prefer the `diffs` tool instead of w
The `diffs` tool accepts either `before` + `after` text, or a unified `patch` string.
Check `details.changed`: identical before/after input returns `false` without creating an artifact; rendered results return `true`.
Use `mode=view` when you want an interactive gateway-hosted viewer. After the tool returns, use `details.viewerUrl` with the canvas tool via `canvas present` or `canvas navigate`.
If the deployment uses a loopback trusted proxy (for example Tailscale Serve with `gateway.trustedProxies` including `127.0.0.1`), raw loopback viewer requests can fail closed without forwarded client-IP headers. In that topology, prefer `mode=file` / `mode=both`, or use a configured `viewerBaseUrl` / explicit proxy/public `baseUrl` when you need a shareable viewer URL.
+76
View File
@@ -208,6 +208,81 @@ describe("PlaywrightDiffScreenshotter", () => {
expect(pages).toHaveLength(1);
expect(pages[0]?.screenshot).toHaveBeenCalledTimes(0);
});
it("wraps browser launch failures with Chromium installation guidance", async () => {
launchMock.mockRejectedValue(new Error("launch failed"));
const screenshotter = new PlaywrightDiffScreenshotter({
config: createConfig(),
browserIdleMs: 1_000,
});
await expect(
screenshotter.screenshotHtml({
html: '<html><head></head><body><main class="oc-frame"></main></body></html>',
outputPath,
theme: "dark",
image: {
format: "png",
qualityPreset: "standard",
scale: 2,
maxWidth: 960,
maxPixels: 8_000_000,
},
}),
).rejects.toThrow("requires a Chromium-compatible browser");
});
it("wraps new-page failures with Chromium installation guidance", async () => {
const browser = createMockBrowser([]);
browser.newPage.mockRejectedValue(new Error("page creation failed"));
launchMock.mockResolvedValue(browser);
const screenshotter = new PlaywrightDiffScreenshotter({
config: createConfig(),
browserIdleMs: 1_000,
});
await expect(
screenshotter.screenshotHtml({
html: '<html><head></head><body><main class="oc-frame"></main></body></html>',
outputPath,
theme: "dark",
image: {
format: "png",
qualityPreset: "standard",
scale: 2,
maxWidth: 960,
maxPixels: 8_000_000,
},
}),
).rejects.toThrow("requires a Chromium-compatible browser");
});
it("preserves render errors after a browser page has opened", async () => {
const browser = createMockBrowser([]);
const page = createMockPage();
page.waitForFunction.mockRejectedValue(new Error("hydration timeout"));
browser.newPage.mockResolvedValue(page);
launchMock.mockResolvedValue(browser);
const screenshotter = new PlaywrightDiffScreenshotter({
config: createConfig(),
browserIdleMs: 1_000,
});
await expect(
screenshotter.screenshotHtml({
html: '<html><head></head><body><main class="oc-frame"></main></body></html>',
outputPath,
theme: "dark",
image: {
format: "png",
qualityPreset: "standard",
scale: 2,
maxWidth: 960,
maxPixels: 8_000_000,
},
}),
).rejects.toThrow("hydration timeout");
});
});
describe("diffs plugin registration", () => {
@@ -428,6 +503,7 @@ describe("diffs plugin registration", () => {
[
"When you need to show edits as a real diff, prefer the `diffs` tool instead of writing a manual summary.",
"It accepts either `before` + `after` text or a unified `patch`.",
"Check `details.changed`: identical before/after input returns `false` without creating an artifact; rendered results return `true`.",
"`mode=view` returns `details.viewerUrl` for canvas use; `mode=file` returns `details.filePath`; `mode=both` returns both.",
"If you need to send the rendered file, use the `message` tool with `path` or `filePath`.",
"Include `path` when you know the filename, and omit presentation overrides unless needed.",
+30 -21
View File
@@ -68,24 +68,33 @@ export class PlaywrightDiffScreenshotter implements DiffScreenshotter {
theme: DiffTheme;
image: DiffRenderOptions["image"];
}): Promise<string> {
const lease = await acquireSharedBrowser({
config: this.config,
idleMs: this.browserIdleMs,
});
let lease: BrowserLease;
try {
lease = await acquireSharedBrowser({
config: this.config,
idleMs: this.browserIdleMs,
});
} catch (error) {
throw buildBrowserUnavailableError(error);
}
let page: Awaited<ReturnType<BrowserInstance["newPage"]>> | undefined;
let currentScale = params.image.scale;
const maxRetries = 2;
try {
for (let attempt = 0; attempt <= maxRetries; attempt += 1) {
page = await lease.browser.newPage({
viewport: {
width: Math.max(Math.ceil(params.image.maxWidth + 240), 1200),
height: 900,
},
deviceScaleFactor: currentScale,
colorScheme: params.theme,
});
try {
page = await lease.browser.newPage({
viewport: {
width: Math.max(Math.ceil(params.image.maxWidth + 240), 1200),
height: 900,
},
deviceScaleFactor: currentScale,
colorScheme: params.theme,
});
} catch (error) {
throw buildBrowserUnavailableError(error);
}
await page.route("**/*", async (route) => {
const requestUrl = route.request().url();
if (requestUrl === "about:blank" || requestUrl.startsWith("data:")) {
@@ -276,15 +285,6 @@ export class PlaywrightDiffScreenshotter implements DiffScreenshotter {
return params.outputPath;
}
throw new Error(IMAGE_SIZE_LIMIT_ERROR);
} catch (error) {
if (error instanceof Error && error.message === IMAGE_SIZE_LIMIT_ERROR) {
throw error;
}
const reason = formatErrorMessage(error);
throw new Error(
`Diff PNG/PDF rendering requires a Chromium-compatible browser. Set browser.executablePath or install Chrome/Chromium. ${reason}`,
{ cause: error },
);
} finally {
await page?.close().catch(() => {});
await lease.release();
@@ -292,6 +292,14 @@ export class PlaywrightDiffScreenshotter implements DiffScreenshotter {
}
}
function buildBrowserUnavailableError(error: unknown): Error {
const reason = formatErrorMessage(error);
return new Error(
`Diff PNG/PDF rendering requires a Chromium-compatible browser. Set browser.executablePath or install Chrome/Chromium. ${reason}`,
{ cause: error },
);
}
async function writeExternalArtifactFile(params: {
outputPath: string;
write: (tempPath: string) => Promise<void>;
@@ -449,6 +457,7 @@ function scheduleIdleBrowserClose(state: SharedBrowserState, idleMs: number): vo
void closeSharedBrowser();
}
}, idleMs);
state.idleTimer.unref();
}
function clearIdleTimer(state: SharedBrowserState): void {
+31 -11
View File
@@ -62,7 +62,7 @@ function compileManifestConfigSchema() {
schema: manifest.configSchema,
value,
applyDefaults: true,
}).ok;
});
}
function requireRecord(value: unknown, label: string): Record<string, unknown> {
@@ -216,6 +216,20 @@ describe("resolveDiffsPluginDefaults", () => {
);
});
it("prefers an explicit canonical default value over a deprecated alias", () => {
expectFields(
resolveDiffsPluginDefaults({
defaults: {
fileFormat: "png",
imageFormat: "pdf",
},
}),
{
fileFormat: "png",
},
);
});
it("accepts plugin-wide artifact TTL defaults", () => {
expectFields(
resolveDiffsPluginDefaults({
@@ -240,7 +254,7 @@ describe("resolveDiffsPluginDefaults", () => {
);
});
it("keeps loader-applied schema defaults from shadowing aliases and quality-derived defaults", () => {
it("keeps alias-only config values after manifest validation", () => {
const validate = compileManifestConfigSchema();
const aliasOnly = {
@@ -249,8 +263,11 @@ describe("resolveDiffsPluginDefaults", () => {
imageQuality: "hq",
},
};
expect(validate(aliasOnly)).toBe(true);
expectFields(resolveDiffsPluginDefaults(aliasOnly), {
const validatedAliasOnly = validate(aliasOnly);
if (!validatedAliasOnly.ok) {
throw new Error("Expected alias-only config to pass manifest validation.");
}
expectFields(resolveDiffsPluginDefaults(validatedAliasOnly.value), {
fileFormat: "pdf",
fileQuality: "hq",
fileScale: 2.5,
@@ -262,8 +279,11 @@ describe("resolveDiffsPluginDefaults", () => {
fileQuality: "hq",
},
};
expect(validate(qualityOnly)).toBe(true);
expectFields(resolveDiffsPluginDefaults(qualityOnly), {
const validatedQualityOnly = validate(qualityOnly);
if (!validatedQualityOnly.ok) {
throw new Error("Expected quality-only config to pass manifest validation.");
}
expectFields(resolveDiffsPluginDefaults(validatedQualityOnly.value), {
fileQuality: "hq",
fileScale: 2.5,
fileMaxWidth: 1200,
@@ -301,10 +321,10 @@ describe("diffs plugin schema surfaces", () => {
it("rejects invalid viewerBaseUrl values at manifest-validation time too", () => {
const validate = compileManifestConfigSchema();
expect(validate({ viewerBaseUrl: "javascript:alert(1)" })).toBe(false);
expect(validate({ viewerBaseUrl: "https://example.com/openclaw?x=1" })).toBe(false);
expect(validate({ viewerBaseUrl: "https://example.com/openclaw#frag" })).toBe(false);
expect(validate({ viewerBaseUrl: "https://example.com/openclaw/" })).toBe(true);
expect(validate({ viewerBaseUrl: "javascript:alert(1)" }).ok).toBe(false);
expect(validate({ viewerBaseUrl: "https://example.com/openclaw?x=1" }).ok).toBe(false);
expect(validate({ viewerBaseUrl: "https://example.com/openclaw#frag" }).ok).toBe(false);
expect(validate({ viewerBaseUrl: "https://example.com/openclaw/" }).ok).toBe(true);
});
it("preserves defaults and security for direct safeParse callers", () => {
@@ -344,7 +364,7 @@ describe("diffs plugin schema surfaces", () => {
expectFields(data.security, { allowRemoteViewer: true });
});
it("canonicalizes alias-driven defaults for direct safeParse callers", () => {
it("resolves deprecated aliases before safeParse applies runtime defaults", () => {
const parsed = requireRecord(
diffsPluginConfigSchema.safeParse?.({
defaults: {
+2 -14
View File
@@ -144,15 +144,9 @@ const DiffsPluginJsonSchemaSource = z.strictObject({
wordWrap: z.boolean().default(DEFAULT_DIFFS_TOOL_DEFAULTS.wordWrap).optional(),
background: z.boolean().default(DEFAULT_DIFFS_TOOL_DEFAULTS.background).optional(),
theme: z.enum(DIFF_THEMES).default(DEFAULT_DIFFS_TOOL_DEFAULTS.theme).optional(),
fileFormat: z
.enum(DIFF_OUTPUT_FORMATS)
.default(DEFAULT_DIFFS_TOOL_DEFAULTS.fileFormat)
.optional(),
fileFormat: z.enum(DIFF_OUTPUT_FORMATS).optional(),
format: z.enum(DIFF_OUTPUT_FORMATS).optional().describe("Deprecated alias for fileFormat."),
fileQuality: z
.enum(DIFF_IMAGE_QUALITY_PRESETS)
.default(DEFAULT_DIFFS_TOOL_DEFAULTS.fileQuality)
.optional(),
fileQuality: z.enum(DIFF_IMAGE_QUALITY_PRESETS).optional(),
fileScale: z.number().min(1).max(4).optional(),
fileMaxWidth: z.number().min(640).max(2400).optional(),
imageFormat: z
@@ -225,12 +219,8 @@ export const diffsPluginConfigSchema: OpenClawPluginConfigSchema = {
function resolveConfiguredValue<T>(options: {
primary: T | undefined;
aliases: Array<T | undefined>;
schemaDefault?: T;
}): T | undefined {
const alias = options.aliases.find((value): value is T => value !== undefined);
if (alias !== undefined && options.primary === options.schemaDefault) {
return alias;
}
return options.primary ?? alias;
}
@@ -257,14 +247,12 @@ export function resolveDiffsPluginDefaults(config: unknown): DiffToolDefaults {
resolveConfiguredValue({
primary: defaults.fileQuality,
aliases: [defaults.imageQuality],
schemaDefault: DEFAULT_DIFFS_TOOL_DEFAULTS.fileQuality,
}),
);
const profile = DEFAULT_IMAGE_QUALITY_PROFILES[fileQuality];
const fileFormat = resolveConfiguredValue({
primary: defaults.fileFormat,
aliases: [defaults.imageFormat, defaults.format],
schemaDefault: DEFAULT_DIFFS_TOOL_DEFAULTS.fileFormat,
});
const fileScale = resolveConfiguredValue({
primary: defaults.fileScale,
+13 -4
View File
@@ -5,7 +5,7 @@ import type { PluginLogger } from "../api.js";
import { resolveRequestClientIp } from "../runtime-api.js";
import type { DiffArtifactStore } from "./store.js";
import { DIFF_ARTIFACT_ID_PATTERN, DIFF_ARTIFACT_TOKEN_PATTERN } from "./types.js";
import { VIEWER_ASSET_PREFIX, getServedViewerAsset } from "./viewer-assets.js";
import { VIEWER_ASSET_PREFIX, VIEWER_RUNTIME_PATH, getServedViewerAsset } from "./viewer-assets.js";
const VIEW_PREFIX = "/plugins/diffs/view/";
const VIEWER_MAX_FAILURES_PER_WINDOW = 40;
@@ -23,6 +23,7 @@ const VIEWER_CONTENT_SECURITY_POLICY = [
"frame-ancestors 'self'",
"object-src 'none'",
].join("; ");
const IMMUTABLE_ASSET_CACHE_CONTROL = "public, max-age=31536000, immutable";
export function createDiffsHttpHandler(params: {
store: DiffArtifactStore;
@@ -154,7 +155,11 @@ async function serveAsset(
}
res.statusCode = 200;
setSharedHeaders(res, asset.contentType);
setSharedHeaders(
res,
asset.contentType,
pathname === VIEWER_RUNTIME_PATH ? IMMUTABLE_ASSET_CACHE_CONTROL : undefined,
);
if (req.method === "HEAD") {
res.end();
} else {
@@ -174,8 +179,12 @@ function respondText(res: ServerResponse, statusCode: number, body: string): voi
res.end(body);
}
function setSharedHeaders(res: ServerResponse, contentType: string): void {
res.setHeader("cache-control", "no-store, max-age=0");
function setSharedHeaders(
res: ServerResponse,
contentType: string,
cacheControl = "no-store, max-age=0",
): void {
res.setHeader("cache-control", cacheControl);
res.setHeader("content-type", contentType);
res.setHeader("x-content-type-options", "nosniff");
res.setHeader("referrer-policy", "no-referrer");
+11 -11
View File
@@ -19,6 +19,16 @@ describe("normalizeSupportedLanguageHint", () => {
]);
});
it("normalizes language hint casing", async () => {
await expect(normalizeHints(["Python", "TypeScript"])).resolves.toEqual([
"python",
"typescript",
]);
await expect(
normalizeSupportedLanguageHint("AbAp", { languagePackAvailable: true }),
).resolves.toBe("abap");
});
it("normalizes common aliases to base viewer languages", async () => {
await expect(
normalizeHints(["ts", "c++", "c#", "bash", "dockerfile", "rb", "kt", "ps1"]),
@@ -36,17 +46,7 @@ describe("normalizeSupportedLanguageHint", () => {
it("keeps mainstream languages in the base viewer without the language pack", async () => {
await expect(
normalizeHints([
"ruby",
"swift",
"kotlin",
"r",
"dart",
"lua",
"powershell",
"xml",
"toml",
]),
normalizeHints(["ruby", "swift", "kotlin", "r", "dart", "lua", "powershell", "xml", "toml"]),
).resolves.toEqual([
"ruby",
"swift",
+2 -2
View File
@@ -26,8 +26,8 @@ function normalizeOptionalString(value: unknown): string | undefined {
if (typeof value !== "string") {
return undefined;
}
const trimmed = value.trim();
return trimmed ? trimmed : undefined;
const normalized = value.trim().toLowerCase();
return normalized ? normalized : undefined;
}
export async function normalizeSupportedLanguageHint(
+1
View File
@@ -2,6 +2,7 @@
export const DIFFS_AGENT_GUIDANCE = [
"When you need to show edits as a real diff, prefer the `diffs` tool instead of writing a manual summary.",
"It accepts either `before` + `after` text or a unified `patch`.",
"Check `details.changed`: identical before/after input returns `false` without creating an artifact; rendered results return `true`.",
"`mode=view` returns `details.viewerUrl` for canvas use; `mode=file` returns `details.filePath`; `mode=both` returns both.",
"If you need to send the rendered file, use the `message` tool with `path` or `filePath`.",
"Include `path` when you know the filename, and omit presentation overrides unless needed.",
+1 -1
View File
@@ -72,7 +72,7 @@ describe("renderDiffDocument render targets", () => {
expect(rendered.html).toContain("mock diff");
expect(rendered.imageHtml).toContain("mock diff");
expect(preloadMultiFileDiffMock).toHaveBeenCalledTimes(2);
expect(preloadMultiFileDiffMock).toHaveBeenCalledTimes(1);
});
it("renders only the image variant for patch image mode", async () => {
@@ -0,0 +1,70 @@
// Diffs tests cover shared SSR preload behavior.
import { disposeHighlighter } from "@pierre/diffs";
import * as diffsSsr from "@pierre/diffs/ssr";
import { afterEach, describe, expect, it, vi } from "vitest";
import { DEFAULT_DIFFS_TOOL_DEFAULTS, resolveDiffImageRenderOptions } from "./config.js";
import { renderDiffDocument } from "./render.js";
vi.mock("@pierre/diffs/ssr", async (importOriginal) => {
const actual = await importOriginal<typeof import("@pierre/diffs/ssr")>();
return {
...actual,
preloadFileDiff: vi.fn(actual.preloadFileDiff),
preloadMultiFileDiff: vi.fn(actual.preloadMultiFileDiff),
};
});
describe("renderDiffDocument SSR preloads", () => {
afterEach(async () => {
vi.clearAllMocks();
await disposeHighlighter();
});
it("preloads a before/after diff once for viewer and image output", async () => {
await renderDiffDocument(
{
kind: "before_after",
before: "const value = 1;\n",
after: "const value = 2;\n",
path: "src/example.ts",
},
{
presentation: DEFAULT_DIFFS_TOOL_DEFAULTS,
image: resolveDiffImageRenderOptions({ defaults: DEFAULT_DIFFS_TOOL_DEFAULTS }),
expandUnchanged: false,
},
"both",
);
expect(diffsSsr.preloadMultiFileDiff).toHaveBeenCalledTimes(1);
});
it("preloads each patch file once for viewer and image output", async () => {
const patch = [
"diff --git a/a.ts b/a.ts",
"--- a/a.ts",
"+++ b/a.ts",
"@@ -1 +1 @@",
"-const a = 1;",
"+const a = 2;",
"diff --git a/b.ts b/b.ts",
"--- a/b.ts",
"+++ b/b.ts",
"@@ -1 +1 @@",
"-const b = 1;",
"+const b = 2;",
].join("\n");
await renderDiffDocument(
{ kind: "patch", patch },
{
presentation: DEFAULT_DIFFS_TOOL_DEFAULTS,
image: resolveDiffImageRenderOptions({ defaults: DEFAULT_DIFFS_TOOL_DEFAULTS }),
expandUnchanged: false,
},
"both",
);
expect(diffsSsr.preloadFileDiff).toHaveBeenCalledTimes(2);
});
});
+91 -93
View File
@@ -24,6 +24,13 @@ const MAX_PATCH_TOTAL_LINES = 120_000;
const VIEWER_LOADER_DOCUMENT_PATH = "../../assets/viewer.js";
const LANGUAGE_PACK_VIEWER_LOADER_DOCUMENT_PATH = "../../../diffs-language-pack/assets/viewer.js";
export class DiffRenderInputError extends Error {
constructor(message: string) {
super(message);
this.name = "DiffRenderInputError";
}
}
function escapeCssString(value: string): string {
return value.replaceAll("\\", "\\\\").replaceAll('"', '\\"');
}
@@ -65,11 +72,19 @@ function resolveBeforeAfterFileName(params: {
return DEFAULT_FILE_NAME;
}
function resolveDiffTypography(presentation: DiffRenderOptions["presentation"]): {
fontSize: number;
lineHeight: number;
} {
const fontSize = normalizeDiffFontSize(presentation.fontSize);
const lineSpacing = normalizeDiffLineSpacing(presentation.lineSpacing);
const lineHeight = Math.max(20, Math.round(fontSize * lineSpacing));
return { fontSize, lineHeight };
}
function buildDiffOptions(options: DiffRenderOptions): DiffViewerOptions {
const fontFamily = escapeCssString(options.presentation.fontFamily);
const fontSize = normalizeDiffFontSize(options.presentation.fontSize);
const lineSpacing = normalizeDiffLineSpacing(options.presentation.lineSpacing);
const lineHeight = Math.max(20, Math.round(fontSize * lineSpacing));
const { fontSize, lineHeight } = resolveDiffTypography(options.presentation);
return {
theme: {
light: "pierre-light",
@@ -201,6 +216,10 @@ function buildHtmlDocument(params: {
bodyHtml: string;
theme: DiffRenderOptions["presentation"]["theme"];
imageMaxWidth: number;
imageTypography: {
fontSize: number;
lineHeight: number;
};
runtimeMode: "viewer" | "image";
viewerRuntime: "base" | "language-pack";
}): string {
@@ -208,6 +227,15 @@ function buildHtmlDocument(params: {
params.viewerRuntime === "language-pack"
? LANGUAGE_PACK_VIEWER_LOADER_DOCUMENT_PATH
: VIEWER_LOADER_DOCUMENT_PATH;
const imageTypographyCss =
params.runtimeMode === "image"
? `
.oc-frame[data-render-mode="image"] .oc-diff-host {
--diffs-font-size: ${params.imageTypography.fontSize}px;
--diffs-line-height: ${params.imageTypography.lineHeight}px;
}
`
: "";
return `<!doctype html>
<html lang="en">
<head>
@@ -256,6 +284,7 @@ function buildHtmlDocument(params: {
.oc-frame[data-render-mode="image"] {
max-width: ${Math.max(640, Math.round(params.imageMaxWidth))}px;
}
${imageTypographyCss}
[data-openclaw-diff-root] {
display: grid;
@@ -364,54 +393,32 @@ async function renderBeforeAfterDiff(
...(lang ? { lang } : {}),
};
const { viewerOptions, imageOptions } = buildRenderVariants({ options, target });
const [viewerResult, imageResult] = await Promise.all([
viewerOptions
? preloadMultiFileDiffWithFallback({
oldFile,
newFile,
options: viewerOptions,
})
: Promise.resolve(undefined),
imageOptions
? preloadMultiFileDiffWithFallback({
oldFile,
newFile,
options: imageOptions,
})
: Promise.resolve(undefined),
]);
const [viewerPayload, imagePayload] = await Promise.all([
viewerResult && viewerOptions
? normalizeDiffViewerPayloadLanguages(
{
prerenderedHTML: viewerResult.prerenderedHTML,
oldFile: viewerResult.oldFile,
newFile: viewerResult.newFile,
options: viewerOptions,
langs: collectDiffPayloadLanguageHints({
oldFile: viewerResult.oldFile,
newFile: viewerResult.newFile,
}),
},
{ languagePackAvailable },
)
: Promise.resolve(undefined),
imageResult && imageOptions
? normalizeDiffViewerPayloadLanguages(
{
prerenderedHTML: imageResult.prerenderedHTML,
oldFile: imageResult.oldFile,
newFile: imageResult.newFile,
options: imageOptions,
langs: collectDiffPayloadLanguageHints({
oldFile: imageResult.oldFile,
newFile: imageResult.newFile,
}),
},
{ languagePackAvailable },
)
: Promise.resolve(undefined),
]);
const preloadOptions = viewerOptions ?? imageOptions;
if (!preloadOptions) {
throw new Error(`Unsupported diff render target: ${target}`);
}
const preloadResult = await preloadMultiFileDiffWithFallback({
oldFile,
newFile,
options: preloadOptions,
});
const normalizedPayload = await normalizeDiffViewerPayloadLanguages(
{
prerenderedHTML: preloadResult.prerenderedHTML,
oldFile: preloadResult.oldFile,
newFile: preloadResult.newFile,
options: preloadOptions,
langs: collectDiffPayloadLanguageHints({
oldFile: preloadResult.oldFile,
newFile: preloadResult.newFile,
}),
},
{ languagePackAvailable },
);
const viewerPayload = viewerOptions
? { ...normalizedPayload, options: viewerOptions }
: undefined;
const imagePayload = imageOptions ? { ...normalizedPayload, options: imageOptions } : undefined;
const section = buildRenderedSection({
...(viewerPayload ? { viewerPayload } : {}),
...(imagePayload ? { imagePayload } : {}),
@@ -441,10 +448,12 @@ async function renderPatchDiff(
.map((fileDiff) => normalizePatchFileLanguage(fileDiff, { languagePackAvailable })),
);
if (files.length === 0) {
throw new Error("Patch input did not contain any file diffs.");
throw new DiffRenderInputError("Patch input did not contain any file diffs.");
}
if (files.length > MAX_PATCH_FILE_COUNT) {
throw new Error(`Patch input contains too many files (max ${MAX_PATCH_FILE_COUNT}).`);
throw new DiffRenderInputError(
`Patch input contains too many files (max ${MAX_PATCH_FILE_COUNT}).`,
);
}
const totalLines = files.reduce((sum, fileDiff) => {
const splitLines = Number.isFinite(fileDiff.splitLineCount) ? fileDiff.splitLineCount : 0;
@@ -452,51 +461,37 @@ async function renderPatchDiff(
return sum + Math.max(splitLines, unifiedLines, 0);
}, 0);
if (totalLines > MAX_PATCH_TOTAL_LINES) {
throw new Error(`Patch input is too large to render (max ${MAX_PATCH_TOTAL_LINES} lines).`);
throw new DiffRenderInputError(
`Patch input is too large to render (max ${MAX_PATCH_TOTAL_LINES} lines).`,
);
}
const { viewerOptions, imageOptions } = buildRenderVariants({ options, target });
const preloadOptions = viewerOptions ?? imageOptions;
if (!preloadOptions) {
throw new Error(`Unsupported diff render target: ${target}`);
}
const sections = await Promise.all(
files.map(async (fileDiff) => {
const [viewerResult, imageResult] = await Promise.all([
viewerOptions
? preloadFileDiffWithFallback({
fileDiff,
options: viewerOptions,
})
: Promise.resolve(undefined),
imageOptions
? preloadFileDiffWithFallback({
fileDiff,
options: imageOptions,
})
: Promise.resolve(undefined),
]);
const [viewerPayload, imagePayload] = await Promise.all([
viewerResult && viewerOptions
? normalizeDiffViewerPayloadLanguages(
{
prerenderedHTML: viewerResult.prerenderedHTML,
fileDiff: viewerResult.fileDiff,
options: viewerOptions,
langs: collectDiffPayloadLanguageHints({ fileDiff: viewerResult.fileDiff }),
},
{ languagePackAvailable },
)
: Promise.resolve(undefined),
imageResult && imageOptions
? normalizeDiffViewerPayloadLanguages(
{
prerenderedHTML: imageResult.prerenderedHTML,
fileDiff: imageResult.fileDiff,
options: imageOptions,
langs: collectDiffPayloadLanguageHints({ fileDiff: imageResult.fileDiff }),
},
{ languagePackAvailable },
)
: Promise.resolve(undefined),
]);
const preloadResult = await preloadFileDiffWithFallback({
fileDiff,
options: preloadOptions,
});
const normalizedPayload = await normalizeDiffViewerPayloadLanguages(
{
prerenderedHTML: preloadResult.prerenderedHTML,
fileDiff: preloadResult.fileDiff,
options: preloadOptions,
langs: collectDiffPayloadLanguageHints({ fileDiff: preloadResult.fileDiff }),
},
{ languagePackAvailable },
);
const viewerPayload = viewerOptions
? { ...normalizedPayload, options: viewerOptions }
: undefined;
const imagePayload = imageOptions
? { ...normalizedPayload, options: imageOptions }
: undefined;
return buildRenderedSection({
...(viewerPayload ? { viewerPayload } : {}),
@@ -537,6 +532,7 @@ export async function renderDiffDocument(
? await renderBeforeAfterDiff(input, options, target)
: await renderPatchDiff(input, options, target);
const viewerRuntime = rendered.usesLanguagePack ? "language-pack" : "base";
const imageTypography = resolveDiffTypography(buildImageRenderOptions(options).presentation);
return {
...(rendered.viewerBodyHtml
@@ -546,6 +542,7 @@ export async function renderDiffDocument(
bodyHtml: rendered.viewerBodyHtml,
theme: options.presentation.theme,
imageMaxWidth: options.image.maxWidth,
imageTypography,
runtimeMode: "viewer",
viewerRuntime,
}),
@@ -558,6 +555,7 @@ export async function renderDiffDocument(
bodyHtml: rendered.imageBodyHtml,
theme: options.presentation.theme,
imageMaxWidth: options.image.maxWidth,
imageTypography,
runtimeMode: "image",
viewerRuntime,
}),
+5
View File
@@ -53,6 +53,8 @@ describe("DiffArtifactStore", () => {
agentAccountId: "default",
});
expect(await store.readHtml(artifact.id)).toBe("<html>demo</html>");
await expect(store.getArtifact(artifact.id, "0".repeat(48))).resolves.toBeNull();
await expect(store.getArtifact(artifact.id, "short")).resolves.toBeNull();
});
it("caps artifact expiry instead of throwing near the Date boundary", async () => {
@@ -280,6 +282,7 @@ describe("createDiffsHttpHandler", () => {
expect(res.statusCode).toBe(200);
expect(res.body).toBe("<html>viewer</html>");
expect(res.getHeader("content-security-policy")).toContain("default-src 'none'");
expect(res.getHeader("cache-control")).toBe("no-store, max-age=0");
});
it("rejects invalid tokens", async () => {
@@ -321,6 +324,7 @@ describe("createDiffsHttpHandler", () => {
expect(handled).toBe(true);
expect(res.statusCode).toBe(200);
expect(String(res.body)).toContain("./viewer-runtime.js?v=");
expect(res.getHeader("cache-control")).toBe("no-store, max-age=0");
});
it("serves the shared viewer runtime asset", async () => {
@@ -337,6 +341,7 @@ describe("createDiffsHttpHandler", () => {
expect(handled).toBe(true);
expect(res.statusCode).toBe(200);
expect(String(res.body)).toContain("openclawDiffsReady");
expect(res.getHeader("cache-control")).toBe("public, max-age=31536000, immutable");
});
it.each([
+2 -2
View File
@@ -3,7 +3,7 @@ import crypto from "node:crypto";
import fs from "node:fs/promises";
import path from "node:path";
import { MAX_DATE_TIMESTAMP_MS, timestampMsToIsoString } from "openclaw/plugin-sdk/number-runtime";
import { root as fsRoot } from "openclaw/plugin-sdk/security-runtime";
import { root as fsRoot, safeEqualSecret } from "openclaw/plugin-sdk/security-runtime";
import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime";
import type { PluginLogger } from "../api.js";
import type { DiffArtifactContext, DiffArtifactMeta, DiffOutputFormat } from "./types.js";
@@ -94,7 +94,7 @@ export class DiffArtifactStore {
if (!meta) {
return null;
}
if (meta.token !== token) {
if (!safeEqualSecret(token, meta.token)) {
return null;
}
if (isExpired(meta)) {
+55 -1
View File
@@ -12,11 +12,16 @@ import { createDiffsTool } from "./tool.js";
import type { DiffRenderOptions } from "./types.js";
describe("diffs tool", () => {
let rootDir: string;
let store: DiffArtifactStore;
let cleanupRootDir: () => Promise<void>;
beforeEach(async () => {
({ store, cleanup: cleanupRootDir } = await createDiffStoreHarness("openclaw-diffs-tool-"));
({
rootDir,
store,
cleanup: cleanupRootDir,
} = await createDiffStoreHarness("openclaw-diffs-tool-"));
});
afterEach(async () => {
@@ -42,6 +47,32 @@ describe("diffs tool", () => {
expect(String(readDetails(result).viewerUrl)).toContain(
"http://127.0.0.1:18789/plugins/diffs/view/",
);
expect(readDetails(result).changed).toBe(true);
});
it("short-circuits identical before/after input without creating an artifact", async () => {
const screenshotHtml = vi.fn<DiffScreenshotter["screenshotHtml"]>();
const tool = createToolWithScreenshotter(store, { screenshotHtml });
const result = await tool.execute?.("tool-identical", {
before: "same\n",
after: "same\n",
});
expect(readTextContent(result, 0)).toBe(
"Before and after are identical — no changes to render.",
);
expect(readDetails(result)).toEqual({
changed: false,
context: {
agentId: "main",
sessionId: "session-123",
messageChannel: "discord",
agentAccountId: "default",
},
});
expect(screenshotHtml).not.toHaveBeenCalled();
await expect(fs.readdir(rootDir)).resolves.toEqual([]);
});
it("uses configured viewerBaseUrl when tool input omits baseUrl", async () => {
@@ -388,6 +419,29 @@ describe("diffs tool", () => {
).rejects.toThrow("patch exceeds maximum size");
});
it("classifies patch render validation failures as tool input errors", async () => {
const tool = createDiffsTool({
api: createApi(),
store,
defaults: DEFAULT_DIFFS_TOOL_DEFAULTS,
});
const error = await tool
.execute?.("tool-invalid-patch", {
patch: "not a unified patch",
mode: "view",
})
.then(
() => undefined,
(caught: unknown) => caught,
);
expect(error).toMatchObject({
name: "ToolInputError",
message: "Patch input did not contain any file diffs.",
});
});
it("rejects oversized before/after payloads", async () => {
const tool = createDiffsTool({
api: createApi(),
+23 -2
View File
@@ -9,7 +9,7 @@ import type { Static } from "typebox";
import type { AnyAgentTool, OpenClawPluginApi, OpenClawPluginToolContext } from "../api.js";
import { PlaywrightDiffScreenshotter, type DiffScreenshotter } from "./browser.js";
import { resolveDiffImageRenderOptions } from "./config.js";
import { renderDiffDocument } from "./render.js";
import { DiffRenderInputError, renderDiffDocument } from "./render.js";
import type { DiffArtifactStore } from "./store.js";
import type {
DiffArtifactContext,
@@ -167,6 +167,20 @@ export function createDiffsTool(params: {
const rawRecord = rawParams as Record<string, unknown>;
const artifactContext = buildArtifactContext(params.context);
const input = normalizeDiffInput(toolParams);
if (input.kind === "before_after" && input.before === input.after) {
return {
content: [
{
type: "text",
text: "Before and after are identical — no changes to render.",
},
],
details: {
changed: false,
...(artifactContext ? { context: artifactContext } : {}),
},
};
}
const mode = normalizeMode(toolParams.mode, params.defaults.mode);
const theme = normalizeTheme(toolParams.theme, params.defaults.theme);
const layout = normalizeLayout(toolParams.layout, params.defaults.layout);
@@ -204,7 +218,12 @@ export function createDiffsTool(params: {
languagePackAvailable: params.languagePackAvailable,
},
renderTarget,
);
).catch((error: unknown) => {
if (error instanceof DiffRenderInputError) {
throw new PluginToolInputError(error.message);
}
throw error;
});
const screenshotter =
params.screenshotter ?? new PlaywrightDiffScreenshotter({ config: params.api.config });
@@ -232,6 +251,7 @@ export function createDiffsTool(params: {
],
details: buildArtifactDetails({
baseDetails: {
changed: true,
...(artifactFile.artifactId ? { artifactId: artifactFile.artifactId } : {}),
...(artifactFile.expiresAt ? { expiresAt: artifactFile.expiresAt } : {}),
title: rendered.title,
@@ -262,6 +282,7 @@ export function createDiffsTool(params: {
});
const baseDetails = {
changed: true,
artifactId: artifact.id,
viewerUrl,
viewerPath: artifact.viewerPath,
@@ -192,6 +192,16 @@ describe("hydrateViewer", () => {
});
});
describe("resolveViewerLanguagePackAvailability", () => {
it("resolves defined and undefined build flags", async () => {
const { resolveViewerLanguagePackAvailability } = await import("./viewer-client.js");
expect(resolveViewerLanguagePackAvailability(true)).toBe(true);
expect(resolveViewerLanguagePackAvailability(false)).toBe(false);
expect(resolveViewerLanguagePackAvailability(undefined)).toBe(false);
});
});
describe("viewerState initialization", () => {
beforeEach(() => {
document.body.innerHTML = "";
+19 -1
View File
@@ -10,6 +10,23 @@ import { normalizeDiffViewerPayloadLanguages } from "./language-hints.js";
import type { DiffViewerPayload, DiffLayout, DiffTheme } from "./types.js";
import { parseViewerPayloadJson } from "./viewer-payload.js";
// oxlint-disable-next-line eslint/no-underscore-dangle -- Bundled builds replace this compile-time define identifier.
declare const __OPENCLAW_DIFFS_LANGUAGE_PACK__: boolean | undefined;
// Build-time esbuild define; typeof guard keeps the module loadable where the
// define is absent (vitest/node), matching the __OPENCLAW_VERSION__ pattern.
function readInjectedLanguagePackFlag(): boolean | undefined {
return typeof __OPENCLAW_DIFFS_LANGUAGE_PACK__ === "boolean"
? __OPENCLAW_DIFFS_LANGUAGE_PACK__
: undefined;
}
export function resolveViewerLanguagePackAvailability(
buildFlag: boolean | undefined = readInjectedLanguagePackFlag(),
): boolean {
return buildFlag === true;
}
type ViewerState = {
theme: DiffTheme;
layout: DiffLayout;
@@ -290,10 +307,11 @@ export async function hydrateViewer(): Promise<void> {
// Rehydration replaces the current DOM card set; do not retain controllers
// from a previous render because they can keep stale DOM references alive.
controllers.length = 0;
const languagePackAvailable = resolveViewerLanguagePackAvailability();
const cards = await Promise.all(
getCards().map(async ({ host, payload }) => ({
host,
payload: await normalizeDiffViewerPayloadLanguages(payload),
payload: await normalizeDiffViewerPayloadLanguages(payload, { languagePackAvailable }),
})),
);
const langs = new Set<SupportedLanguages>();
+3
View File
@@ -16,10 +16,12 @@ const targets = {
entry: "extensions/diffs/src/viewer-client.ts",
output: "extensions/diffs/assets/viewer-runtime.js",
shikiAlias: "scripts/diffs-shiki-curated.ts",
languagePackAvailable: false,
},
full: {
entry: "extensions/diffs/src/viewer-client.ts",
output: "extensions/diffs-language-pack/assets/viewer-runtime.js",
languagePackAvailable: true,
},
};
@@ -81,6 +83,7 @@ export async function buildDiffsViewerRuntime(targetName) {
format: "esm",
minify: true,
define: {
__OPENCLAW_DIFFS_LANGUAGE_PACK__: String(target.languagePackAvailable),
NaN: "Number.NaN",
},
legalComments: "none",
+1
View File
@@ -188,6 +188,7 @@ describe("production lint suppressions", () => {
"extensions/browser/src/browser/pw-tools-core.interactions.ts|@typescript-eslint/no-implied-eval|2",
"extensions/browser/src/cli/browser-cli-actions-input/register.files-downloads.ts|typescript/no-unnecessary-type-parameters|1",
"extensions/browser/src/node-host/invoke-browser.ts|typescript/no-unnecessary-type-parameters|1",
"extensions/diffs/src/viewer-client.ts|eslint/no-underscore-dangle|1",
"extensions/discord/src/outbound-adapter.test-harness.ts|typescript/no-unnecessary-type-parameters|1",
"extensions/discord/src/test-support/provider.test-support.ts|typescript/no-unnecessary-type-parameters|1",
"extensions/feishu/src/bitable.ts|typescript/no-unnecessary-type-parameters|1",