diff --git a/packages/tool-server/src/chromium-server/screenshot.ts b/packages/tool-server/src/chromium-server/screenshot.ts index 5c09f7f65..cfada2649 100644 --- a/packages/tool-server/src/chromium-server/screenshot.ts +++ b/packages/tool-server/src/chromium-server/screenshot.ts @@ -102,11 +102,15 @@ export async function captureScreenshot( const rotation = opts.rotation && opts.rotation !== "Portrait" ? opts.rotation : null; const scale = opts.scale != null && opts.scale > 0 && opts.scale < 1 ? opts.scale : null; + const dropped: ("rotation" | "scale")[] = []; + if (rotation || scale) { const sharp = tryLoadSharp(); if (!sharp) { const features = [rotation && "rotation", scale && "scale"].filter(Boolean).join(" + "); warnSharpMissingOnce(features); + if (rotation) dropped.push("rotation"); + if (scale) dropped.push("scale"); } else { let pipeline = sharp(bytes); if (rotation) pipeline = pipeline.rotate(ROTATION_DEGREES[rotation]); @@ -122,6 +126,10 @@ export async function captureScreenshot( kernel: DOWNSCALER_TO_KERNEL[opts.downscaler ?? "lanczos3"], fit: "fill", }); + } else { + // Header unreadable: rotation below still applies, but the resize + // cannot be sized, so the scale the caller asked for is lost. + dropped.push("scale"); } } // The newer @types/node strictly types `Buffer` while @@ -135,13 +143,17 @@ export async function captureScreenshot( const safeDeviceId = ctx.deviceId.replace(/[^A-Za-z0-9_-]/g, "_"); const filePath = path.join(mediaDir(), `argent-screenshot-${safeDeviceId}-${stem}.png`); fs.writeFileSync(filePath, bytes); - return { url: `file://${filePath}`, path: filePath }; + return { + url: `file://${filePath}`, + path: filePath, + ...(dropped.length > 0 ? { droppedFeatures: dropped } : {}), + }; } /** * Read width / height from a PNG IHDR chunk without spinning up a decoder. - * Returns null on a malformed or non-PNG buffer — the caller falls back to - * sharp metadata in that case (which costs a roundtrip but always works). + * Returns null on a malformed or non-PNG buffer. The caller skips the resize in + * that case and reports `scale` as dropped — there is no metadata fallback. */ function readPngSize(buf: Buffer): { width: number; height: number } | null { // PNG signature: 89 50 4E 47 0D 0A 1A 0A diff --git a/packages/tool-server/src/chromium-server/types.ts b/packages/tool-server/src/chromium-server/types.ts index 2e0334038..1b4a30a48 100644 --- a/packages/tool-server/src/chromium-server/types.ts +++ b/packages/tool-server/src/chromium-server/types.ts @@ -57,6 +57,13 @@ export interface MediaReady { url: string; /** Absolute path on the tool-server host. */ path: string; + /** + * Geometry the caller asked for that this capture could not apply — the + * optional `sharp` package is missing, or the PNG header could not be read. + * The image is still returned untouched; the tool turns this into a note so + * the omission is visible to the caller rather than only on stderr. + */ + droppedFeatures?: ("rotation" | "scale")[]; } export interface ViewportSize { diff --git a/packages/tool-server/src/http.ts b/packages/tool-server/src/http.ts index 650edcd77..7939da153 100644 --- a/packages/tool-server/src/http.ts +++ b/packages/tool-server/src/http.ts @@ -50,6 +50,7 @@ import { createChromiumServerRouter, } from "./chromium-server/http-api"; import { resolveDevice as resolveDeviceForWs } from "./utils/device-info"; +import { RESULT_NOTE_KEY } from "./tools/screenshot/dropped-geometry"; const AUTO_SUPPRESS_MS = 30 * 60 * 1000; // 30 minutes @@ -335,6 +336,20 @@ const MAX_UPLOAD_STREAM_BYTES = 2 * 1024 * 1024 * 1024; // 2 GiB // Bounds the total of unconsumed uploads, so many small ones can't do the same. const MAX_PENDING_UPLOAD_BYTES = 8 * 1024 * 1024 * 1024; // 8 GiB +/** + * Pull a tool's per-call note off its result so it can ride the response + * envelope. Mutates `data` so the reserved key never reaches the client, where + * it would otherwise surface in `--json` output and in non-image results. + */ +function takeToolNote(data: unknown): string | undefined { + if (typeof data !== "object" || data === null) return undefined; + const bag = data as Record; + const note = bag[RESULT_NOTE_KEY]; + if (typeof note !== "string" || note.length === 0) return undefined; + delete bag[RESULT_NOTE_KEY]; + return note; +} + export function createHttpApp(registry: Registry, options?: HttpAppOptions): HttpAppHandle { const app = express(); // 48mb: file-input wrappers may inline base64 file content (saved PNG @@ -884,6 +899,12 @@ export function createHttpApp(registry: Registry, options?: HttpAppOptions): Htt if (activeRecordings.length > 0) { notes.push(buildScreenRecordingNote(activeRecordings, Date.now())); } + // A tool can raise a per-call note by returning this reserved key. It + // rides the envelope rather than the result body because clients render + // image results as image blocks plus a "Saved:" line and drop every + // other field — a note inside `data` would never be seen. + const toolNote = takeToolNote(data); + if (toolNote) notes.push(toolNote); const notePayload = notes.length > 0 ? { note: notes.join("\n\n") } : {}; if (wantsStream) { writeLine({ event: "result", data, ...notePayload }); diff --git a/packages/tool-server/src/tools/screenshot/dropped-geometry.ts b/packages/tool-server/src/tools/screenshot/dropped-geometry.ts new file mode 100644 index 000000000..28cb97dcb --- /dev/null +++ b/packages/tool-server/src/tools/screenshot/dropped-geometry.ts @@ -0,0 +1,67 @@ +/** + * Notes for geometry the caller asked for that a capture could not apply. + * + * `screenshot` takes `scale` and `rotation` on every target, but several + * backends cannot honour them: Chromium needs the optional `sharp` package, + * and the Apple TV and Vega captures have no rotation step at all. Without a + * note the caller gets a full-size, un-rotated PNG that looks like a successful + * transform, and the only existing signal — a stderr line — is written once per + * process and never reaches whoever asked. + */ +export type DroppedGeometry = "rotation" | "scale"; + +/** Reserved result key hoisted into the response envelope's `note` by http.ts. */ +export const RESULT_NOTE_KEY = "__argentNote"; + +/** + * Which of the caller's geometry parameters were requested but not applied. + * + * A parameter counts only when the caller actually asked for something the + * backend then ignored. Two no-ops are deliberately not reported: `rotation: + * "Portrait"` is the identity rotation, and `scale: 1` is full size — a visual + * snapshot passes `scale: 1` on every step, and flagging that would attach a + * note to captures where nothing was lost. The `ARGENT_SCREENSHOT_SCALE` + * default is likewise never reported, because the caller never asked for it. + */ +export function requestedGeometry(params: { + rotation?: string | undefined; + scale?: number | undefined; +}): DroppedGeometry[] { + const requested: DroppedGeometry[] = []; + if (params.rotation !== undefined && params.rotation !== "Portrait") requested.push("rotation"); + if (params.scale !== undefined && params.scale > 0 && params.scale < 1) requested.push("scale"); + return requested; +} + +function list(dropped: DroppedGeometry[]): string { + return dropped.length === 2 ? "scale and rotation" : dropped[0]!; +} + +/** + * Chromium can do both transforms — it just needs `sharp`, which is optional + * and not shipped — so this case is worth telling the caller how to fix. + */ +export function chromiumDropNote(dropped: DroppedGeometry[]): string | undefined { + if (dropped.length === 0) return undefined; + return ( + `${list(dropped)} was not applied — this is the unmodified capture. Chromium image ` + + `post-processing needs the optional \`sharp\` package: run \`npm install sharp\` in the ` + + `tool-server's environment and retry, or work with the full-size image.` + ); +} + +/** + * The TV backends have no rotation step. Worded so it does not read as + * retryable — the same call will always come back the same way. + */ +export function unsupportedDropNote( + dropped: DroppedGeometry[], + target: string +): string | undefined { + if (dropped.length === 0) return undefined; + return ( + `${list(dropped)} was not applied — ${target} screenshots cannot be transformed that way, ` + + `so retrying with the same parameter will not change the result. The image is the ` + + `untransformed capture.` + ); +} diff --git a/packages/tool-server/src/tools/screenshot/index.ts b/packages/tool-server/src/tools/screenshot/index.ts index 261430380..5844e4f49 100644 --- a/packages/tool-server/src/tools/screenshot/index.ts +++ b/packages/tool-server/src/tools/screenshot/index.ts @@ -7,6 +7,12 @@ import type { Registry, ToolCapability, ToolDefinition } from "@argent/registry" import { simulatorServerRef, type SimulatorServerApi } from "../../blueprints/simulator-server"; import { chromiumCdpRef, type ChromiumCdpApi } from "../../blueprints/chromium-cdp"; import { resolveDevice } from "../../utils/device-info"; +import { + RESULT_NOTE_KEY, + requestedGeometry, + chromiumDropNote, + unsupportedDropNote, +} from "./dropped-geometry"; import { getScreenshotScale } from "../../utils/simulator-client"; import { captureScreenshotUpright } from "../../utils/rotation-aware-capture"; import { isTvOsSimulator } from "../../utils/ios-devices"; @@ -30,7 +36,9 @@ const zodSchema = z.object({ "current rotation, so it is upright without this. Setting it replaces that with a fixed " + "rotation, which on a rotated device produces an image whose geometry no longer matches " + "`describe` frames or gesture coordinates. On Chromium it rotates the captured image after " + - "Page.captureScreenshot." + "Page.captureScreenshot, which requires the optional `sharp` dependency. Apple TV and Vega " + + "captures cannot be rotated at all. When a rotation is requested but not applied, the " + + "response carries a note saying so — the image is returned unrotated either way." ), scale: z .number() @@ -59,6 +67,13 @@ const zodSchema = z.object({ type Params = z.infer; interface Result { + /** + * Set only when the caller asked for geometry the backend could not apply. + * `http.ts` hoists this reserved key into the response envelope's `note`, + * which every client already renders — the result body itself is discarded + * for image-output tools. + */ + [RESULT_NOTE_KEY]?: string; /** * The captured PNG as an artifact handle. The MCP client materializes it to * a local file and renders it inline — no second fetch of the simulator @@ -155,13 +170,20 @@ Fails if the simulator-server / emulator backend / Chromium CDP is not reachable if (device.platform === "chromium") { const ref = chromiumCdpRef(device); const chromium = (await registry.resolveService(ref.urn, ref.options)) as ChromiumCdpApi; - const { path: capturedPath } = await chromium.captureScreenshot({ + const captured = await chromium.captureScreenshot({ rotation: params.rotation, scale: params.scale, downscaler: params.downscaler, }); - const image = await requireArtifacts(ctx).register(capturedPath, { mimeType: "image/png" }); - return { image }; + const image = await requireArtifacts(ctx).register(captured.path, { + mimeType: "image/png", + }); + // Only report what the caller asked for AND the backend dropped: a + // visual snapshot passes scale 1, which is a no-op, not a loss. + const requested = requestedGeometry(params); + const dropped = (captured.droppedFeatures ?? []).filter((f) => requested.includes(f)); + const note = chromiumDropNote(dropped); + return { image, ...(note ? { [RESULT_NOTE_KEY]: note } : {}) }; } // Distinguish tvOS from iOS by simulator runtime — shape alone can't. @@ -169,7 +191,13 @@ Fails if the simulator-server / emulator backend / Chromium CDP is not reachable if (device.platform === "ios" && (await isTvOsSimulator(params.udid))) { const pngPath = await tvScreenshot(params.udid, scale, signal); const image = await requireArtifacts(ctx).register(pngPath, { mimeType: "image/png" }); - return { image }; + // tvScreenshot has no rotation step at all, so an explicit rotation is + // dropped before any capture happens. + const note = unsupportedDropNote( + requestedGeometry(params).filter((f) => f === "rotation"), + "Apple TV" + ); + return { image, ...(note ? { [RESULT_NOTE_KEY]: note } : {}) }; } // Vega captures host-side via the Android emulator console (`adb emu`) and @@ -178,7 +206,11 @@ Fails if the simulator-server / emulator backend / Chromium CDP is not reachable if (device.platform === "vega") { const pngPath = await captureVegaScreenshotPng({ scale: params.scale }); const image = await requireArtifacts(ctx).register(pngPath, { mimeType: "image/png" }); - return { image }; + const note = unsupportedDropNote( + requestedGeometry(params).filter((f) => f === "rotation"), + "Vega (Fire TV)" + ); + return { image, ...(note ? { [RESULT_NOTE_KEY]: note } : {}) }; } const ref = simulatorServerRef(device); diff --git a/packages/tool-server/test/screenshot-dropped-geometry.test.ts b/packages/tool-server/test/screenshot-dropped-geometry.test.ts new file mode 100644 index 000000000..04c8dc2cd --- /dev/null +++ b/packages/tool-server/test/screenshot-dropped-geometry.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "vitest"; +import { + requestedGeometry, + chromiumDropNote, + unsupportedDropNote, + RESULT_NOTE_KEY, +} from "../src/tools/screenshot/dropped-geometry"; + +describe("requestedGeometry", () => { + it("reports what the caller actually asked for", () => { + expect(requestedGeometry({ rotation: "LandscapeLeft", scale: 0.25 })).toEqual([ + "rotation", + "scale", + ]); + }); + + it("ignores an absent parameter", () => { + // The ARGENT_SCREENSHOT_SCALE default arrives as an absent param, and the + // caller never asked for it — reporting it would put a note on captures + // nobody requested a transform for (auto-screenshot passes only udid). + expect(requestedGeometry({})).toEqual([]); + }); + + it("ignores the identity rotation", () => { + expect(requestedGeometry({ rotation: "Portrait" })).toEqual([]); + }); + + it("ignores a full-size scale", () => { + // A visual snapshot passes scale 1 on every step; nothing is lost there, + // so flagging it would attach a note to every one of them. + expect(requestedGeometry({ scale: 1 })).toEqual([]); + expect(requestedGeometry({ scale: 0 })).toEqual([]); + }); +}); + +describe("drop notes", () => { + it("says nothing when nothing was dropped", () => { + expect(chromiumDropNote([])).toBeUndefined(); + expect(unsupportedDropNote([], "Apple TV")).toBeUndefined(); + }); + + it("tells a Chromium caller how to make it work", () => { + const note = chromiumDropNote(["scale", "rotation"]); + expect(note).toContain("scale and rotation was not applied"); + expect(note).toContain("npm install sharp"); + expect(note).toContain("unmodified capture"); + }); + + it("names the single dropped parameter", () => { + expect(chromiumDropNote(["scale"])).toContain("scale was not applied"); + }); + + it("does not invite a pointless retry on a target that cannot transform", () => { + const note = unsupportedDropNote(["rotation"], "Apple TV"); + expect(note).toContain("Apple TV"); + expect(note).toContain("will not change the result"); + // The Chromium remedy must not leak into a case where it cannot help. + expect(note).not.toContain("npm install"); + }); + + it("keeps the reserved key stable", () => { + // http.ts hoists this off the result into the response envelope; renaming + // it silently drops every note. + expect(RESULT_NOTE_KEY).toBe("__argentNote"); + }); +});