diff --git a/CHANGELOG.md b/CHANGELOG.md index ae218d2..1e1449e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## [0.1.62] - 17-07-2026 + +### Fixed + +- **`browser_snapshot` `prune:true` on modals** — an element is now kept if it is itself visible+focusable (`checkVisibility()`), even under an `aria-hidden` ancestor. Fixes SPA modals rendered under a global `aria-hidden` wrapper returning `count:0`. Only genuinely CSS-hidden or non-focusable decorative nodes under `aria-hidden` are still elided. Default (`prune` off) behavior is unchanged. + +### Added + +- **`browser_screenshot` `path` option** — optional param that writes the PNG/JPEG to disk and returns `path` in `structuredContent` instead of inline base64. Refuses multi-viewport captures (`path_multi_viewport_unsupported`) and mismatched extension/mime (`path_extension_mismatch`). Omitting `path` keeps the existing inline-base64 behavior. + ## [0.1.61] - 17-07-2026 ### Added diff --git a/docs/mcp-tools.md b/docs/mcp-tools.md index 155163a..9f6685b 100644 --- a/docs/mcp-tools.md +++ b/docs/mcp-tools.md @@ -547,7 +547,7 @@ Return the indexed interactive elements of the live page, each with a `ref` to u | --- | --- | --- | --- | | `sessionId` | string | yes | Target session. | | `selectors` | boolean | no | Also return a durable CSS `selector` per element (cacheable to act later without re-snapshotting). | -| `prune` | boolean | no | Drop elements hidden for accessibility (default `false`, output unchanged). When `true`, prunes any element that is `aria-hidden` (self or ancestor), `display:none` (self or ancestor), or `visibility:hidden`/`collapse` — **never** on off-screen, `opacity:0`, or off-viewport position. | +| `prune` | boolean | no | Drop only genuinely hidden or decorative elements (default `false`, output unchanged). When `true`, prunes an element iff it is **CSS-hidden** — `display:none`/`visibility:hidden`/`collapse`/`content-visibility` on itself or an ancestor, via `Element.checkVisibility()` — **or decorative**: under an `aria-hidden="true"` ancestor **and** not focusable. A **visible, focusable** element is **kept even under an `aria-hidden` ancestor**, so the controls of an **open modal** (whose SPA focus-trap marks a root/sibling wrapper `aria-hidden`) stay in the snapshot. Never prunes on off-screen, `opacity:0`, or off-viewport position. | | `annotate` | boolean | no | Also return a Set-of-Marks JPEG: numbered badges (= each `ref`) drawn over the page, for vision models (main-frame, viewport-only). | ```json @@ -748,11 +748,21 @@ Capture the live page as PNG(s) for vision. Pass `ref` for one element, `viewpor | `colorScheme` | enum `light` \| `dark` | no | Emulate `prefers-color-scheme` and toggle `themeClass` on ``, then restore. | | `themeClass` | string | no | Class toggled on `` for class-based dark themes (default `dark`). | | `annotate` | boolean | no | Set-of-Marks JPEG: numbered badges (= each `ref`) over the viewport, for vision models. | +| `path` | string | no | Also write the captured image to disk and return its `path` in `structuredContent`. Single-image captures only. Without `path`, the image is returned base64-inline as before (unchanged). | + +When `path` is set, its extension must match the output mime — `.png` for element/page/multi captures, `.jpg`/`.jpeg` for the `annotate` JPEG. Error codes: + +- `path_multi_viewport_unsupported` — `path` given with `viewports.length > 1` (multiple images); use a single viewport with `path`. +- `path_extension_mismatch` — `path`'s extension does not match the output mime (e.g. `.png` for the `annotate` JPEG, or `.jpg` for a PNG capture). ```json { "sessionId": "s_abc123", "annotate": true, "colorScheme": "dark" } ``` +```json +{ "sessionId": "s_abc123", "fullPage": true, "path": "./shots/home.png" } +``` + ### browser_inspect Computed styles, box model and WCAG text-contrast (AA/AAA) for one element by `ref` — for design review (typography, color, spacing, contrast). Main-frame refs. diff --git a/package.json b/package.json index e722b75..00997e4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@fusengine/browser-mcp", - "version": "0.1.61", + "version": "0.1.62", "description": "MCP server + CLI giving AI agents a real, stealth browser (Patchright/Playwright) — per-country identity, self-healing actions, snapshots, multi-step plans, structured extraction, CDP attach.", "license": "MIT", "author": "Fusengine", @@ -55,7 +55,7 @@ "typecheck": "tsc -p tsconfig.json --noEmit", "lint": "biome check src tests", "test": "bun test tests/unit", - "test:integration": "node --test --import tsx tests/integration/mcp.test.ts tests/integration/probe.test.ts tests/integration/snapshot.test.ts tests/integration/snapshot-frames.test.ts tests/integration/collect.test.ts tests/integration/collect-batch.test.ts tests/integration/selectors.test.ts tests/integration/visual-diff.test.ts tests/integration/session-state.test.ts tests/integration/pipeline.test.ts tests/integration/run.test.ts tests/integration/extract-schema.test.ts tests/integration/recovery.test.ts tests/integration/live-view.test.ts tests/integration/vault.test.ts", + "test:integration": "node --test --import tsx tests/integration/mcp.test.ts tests/integration/probe.test.ts tests/integration/snapshot.test.ts tests/integration/snapshot-prune.test.ts tests/integration/snapshot-frames.test.ts tests/integration/collect.test.ts tests/integration/collect-batch.test.ts tests/integration/selectors.test.ts tests/integration/visual-diff.test.ts tests/integration/session-state.test.ts tests/integration/pipeline.test.ts tests/integration/run.test.ts tests/integration/extract-schema.test.ts tests/integration/recovery.test.ts tests/integration/live-view.test.ts tests/integration/vault.test.ts", "browsers": "patchright install chromium", "mcp": "node --import tsx src/bin/mcp.ts", "cli": "node --import tsx src/bin/cli.ts" diff --git a/src/extraction/snapshot-hidden.ts b/src/extraction/snapshot-hidden.ts index 4cdfc21..4d5fb24 100644 --- a/src/extraction/snapshot-hidden.ts +++ b/src/extraction/snapshot-hidden.ts @@ -1,66 +1,97 @@ /** - * "Hidden for accessibility" detection, mirroring Playwright's - * `isElementHiddenForAria` (packages/injected/src/roleUtils.ts): `aria-hidden` - * and `display:none` are STICKY down the ancestor chain (a descendant cannot - * un-hide itself); `visibility:hidden|collapse` is checked only on the element - * itself (CSS `visibility` inherits, so a descendant's `visibility:visible` - * already resolves through `getComputedStyle`, no manual override needed). - * `display:contents` never counts as `display:none`, so it does not blanket- - * hide the subtree. Deliberately NOT signals here: offscreen position - * (`offsetParent===null`, e.g. `position:fixed`), `opacity:0`, or `sr-only` - * absolute-offscreen patterns — none of those are ARIA-hidden. + * "Prunable" detection for `browser_snapshot`'s `prune:true` (the "C4" rule): + * an element is prunable iff (a) genuinely hidden — `Element.checkVisibility + * ({checkVisibilityCSS, contentVisibilityAuto})` is `false` (display:none + * self/ancestor, content-visibility:hidden, visibility:hidden/collapse) — OR + * (b) under an `aria-hidden="true"` ancestor AND NOT focusable (decorative). + * `checkVisibility()` deliberately ignores `aria-hidden` (pure CSS/box-model + * signal; Baseline Chrome 105/Firefox 106/Safari 17.4), which is exactly why a + * VISIBLE+FOCUSABLE element under an `aria-hidden` ancestor — e.g. an open + * modal ``/`[role=dialog]` whose SPA also marks a sibling/root wrapper + * `aria-hidden` — is KEPT, not pruned. Deliberately NOT used: CDP + * `Accessibility.getFullAXTree` (Chromium-only, would break firefox/webkit). * @module extraction/snapshot-hidden */ /** - * Browser-side definition injected into {@link SNAPSHOT_SCRIPT}: defines - * `isElementHiddenForAria(el)`, embedded the same way `selector.ts` embeds - * `SELECTOR_DEFS`. + * Browser-side definitions injected into {@link SNAPSHOT_SCRIPT}: defines + * `isPrunable(el)`, embedded the same way `selector.ts` embeds `SELECTOR_DEFS`. */ export const HIDDEN_DEFS = ` - const ariaHiddenAncestor = (el) => { + const hasAriaHiddenAncestor = (el) => { let node = el; while (node && node.nodeType === 1) { if (node.getAttribute('aria-hidden') === 'true') return true; - if (getComputedStyle(node).display === 'none') return true; node = node.parentElement || (node.parentNode && node.parentNode.host); } return false; }; - const isElementHiddenForAria = (el) => { - const v = getComputedStyle(el).visibility; - if (v === 'hidden' || v === 'collapse') return true; - return ariaHiddenAncestor(el); + const isFocusable = (el) => { + if (el.closest('[inert]')) return false; + if ('disabled' in el && el.disabled) return false; + const tag = el.tagName; + const native = + (tag === 'A' && el.hasAttribute('href')) || + tag === 'BUTTON' || tag === 'SELECT' || tag === 'TEXTAREA' || tag === 'IFRAME' || + (tag === 'INPUT' && el.type !== 'hidden') || + el.isContentEditable; + const ti = el.getAttribute('tabindex'); + const tabindexOk = ti !== null && !Number.isNaN(parseInt(ti, 10)) && parseInt(ti, 10) >= 0; + if (!native && !tabindexOk) return false; + return el.tabIndex >= 0; + }; + const isPrunable = (el) => { + if (!el.checkVisibility({ checkVisibilityCSS: true, contentVisibilityAuto: true })) return true; + return hasAriaHiddenAncestor(el) && !isFocusable(el); };`; /** - * Minimal ancestor-chain shape mirroring the DOM API surface used by - * {@link HIDDEN_DEFS} above, so the same decision logic is unit-testable - * without a real DOM (same intent as `selector.ts`'s `isStableToken`). + * Minimal ancestor-chain shape mirroring the DOM surface used by {@link + * HIDDEN_DEFS} above, so the same decision logic is unit-testable without a + * real DOM (same intent as `selector.ts`'s `isStableToken`). `focusable` is a + * precomputed stand-in for the browser-side `isFocusable(el)` result — the + * mirror tests the C4 combination logic, not DOM focusability itself. */ -export interface AriaHiddenNode { +export interface PrunableNode { /** This node's own `aria-hidden` attribute value, or `null`/absent. */ ariaHidden?: string | null; /** This node's own computed `display`. */ display?: string; /** This node's own (already-inherited) computed `visibility`. */ visibility?: string; + /** Precomputed focusability of this exact node (native tag/tabindex/disabled/inert). */ + focusable?: boolean; /** Parent in the ancestor chain (host element, for a shadow-root parent). */ - parent?: AriaHiddenNode | null; + parent?: PrunableNode | null; } -/** - * Node-testable mirror of the browser-side `isElementHiddenForAria`: true if - * the node's own resolved `visibility` is hidden/collapse, OR if `aria-hidden` - * or `display:none` appears on the node or any ancestor. - */ -export function isHiddenForAriaMirror(node: AriaHiddenNode): boolean { +/** True if `checkVisibility({checkVisibilityCSS:true})` would resolve to hidden. */ +function isCssHiddenMirror(node: PrunableNode): boolean { if (node.visibility === "hidden" || node.visibility === "collapse") return true; - let cur: AriaHiddenNode | null | undefined = node; + let cur: PrunableNode | null | undefined = node; while (cur) { - if (cur.ariaHidden === "true") return true; if (cur.display === "none") return true; cur = cur.parent; } return false; } + +/** True if `aria-hidden="true"` appears on the node or any ancestor (sticky). */ +function hasAriaHiddenAncestorMirror(node: PrunableNode): boolean { + let cur: PrunableNode | null | undefined = node; + while (cur) { + if (cur.ariaHidden === "true") return true; + cur = cur.parent; + } + return false; +} + +/** + * Node-testable mirror of the browser-side `isPrunable`: true if the node is + * CSS-hidden (display:none anywhere up the chain, or own visibility:hidden/ + * collapse), OR it is under an `aria-hidden` ancestor AND not focusable. + */ +export function isPrunableMirror(node: PrunableNode): boolean { + if (isCssHiddenMirror(node)) return true; + return hasAriaHiddenAncestorMirror(node) && node.focusable !== true; +} diff --git a/src/extraction/snapshot-walk.ts b/src/extraction/snapshot-walk.ts index 1851829..83ad510 100644 --- a/src/extraction/snapshot-walk.ts +++ b/src/extraction/snapshot-walk.ts @@ -49,7 +49,7 @@ export const SNAPSHOT_SCRIPT = `(arg) => { options: el.tagName === 'SELECT' ? [...el.options].slice(0, 12).map((o) => o.label || o.value) : undefined, ariaExpanded: el.getAttribute('aria-expanded'), ariaControls: el.getAttribute('aria-controls'), visible: r.width > 0 && r.height > 0, obscured: obscured(el, r), - ariaHidden: isElementHiddenForAria(el), + prunable: isPrunable(el), selector: wantSel ? genSelector(el) : undefined, box: {x: Math.round(r.x), y: Math.round(r.y), width: Math.round(r.width), height: Math.round(r.height)} }; diff --git a/src/extraction/snapshot.ts b/src/extraction/snapshot.ts index 4fdbd1b..fc03780 100644 --- a/src/extraction/snapshot.ts +++ b/src/extraction/snapshot.ts @@ -18,18 +18,23 @@ export { REF_ATTRIBUTE } from "./snapshot-walk.js"; const MAX_ELEMENTS = 400; /** - * Browser-returned element plus the internal `ariaHidden` scratch flag (see - * `snapshot-hidden.ts`). Never exposed on the final {@link InteractiveElement} - * output — used only to decide pruning, then stripped in {@link captureSnapshot}. + * Browser-returned element plus the internal `prunable` scratch flag (the C4 + * decision from `snapshot-hidden.ts`). Never exposed on the final {@link + * InteractiveElement} output — used only to decide pruning, then stripped in + * {@link captureSnapshot}. */ -type RawElement = InteractiveElement & { ariaHidden?: boolean }; +type RawElement = InteractiveElement & { prunable?: boolean }; /** * Whether a raw element survives pruning: kept unless `prune` is on AND the - * element was flagged hidden-for-accessibility. Exported for unit testing. + * element was flagged prunable (genuinely hidden, or decorative under an + * `aria-hidden` ancestor and not focusable — see `snapshot-hidden.ts`). + * A visible+focusable element under an `aria-hidden` ancestor (e.g. inside an + * open modal dialog) is NOT prunable and is always kept. Exported for unit + * testing. */ -export function shouldKeep(ariaHidden: boolean | undefined, prune: boolean): boolean { - return !(prune && ariaHidden === true); +export function shouldKeep(prunable: boolean | undefined, prune: boolean): boolean { + return !(prune && prunable === true); } /** @@ -37,9 +42,13 @@ export function shouldKeep(ariaHidden: boolean | undefined, prune: boolean): boo * element with a (frame-local) ref attribute and exposing a frame-scoped `ref`. * Detached frames and frames that reject evaluation (e.g. mid-navigation) are * skipped rather than aborting the whole snapshot. When `prune` is `true`, - * elements hidden for accessibility (`aria-hidden`, `display:none`, ancestor- - * hidden, or `visibility:hidden`/`collapse`) are dropped; default `false` - * keeps the output identical to the pre-pruning behavior. + * elements that are genuinely hidden (`Element.checkVisibility()` false — + * `display:none`, `content-visibility:hidden`, `visibility:hidden`/`collapse`) + * OR decorative under an `aria-hidden` ancestor (present but NOT focusable) + * are dropped. A visible AND focusable element under an `aria-hidden` + * ancestor — e.g. an open modal ``/`[role=dialog]` whose SPA also + * marks a sibling/root wrapper `aria-hidden` — is always kept. Default + * `false` keeps the output identical to the pre-pruning behavior. */ export async function captureSnapshot( page: Page, @@ -61,8 +70,8 @@ export async function captureSnapshot( } for (const raw of local) { if (all.length >= MAX_ELEMENTS) break; - const { ariaHidden, ...el } = raw; - if (!shouldKeep(ariaHidden, prune)) continue; + const { prunable, ...el } = raw; + if (!shouldKeep(prunable, prune)) continue; el.ref = f === 0 ? String(el.index) : `${f}:${el.index}`; if (f > 0) el.frame = f; el.index = global++; diff --git a/src/server/tools/screenshot-result.ts b/src/server/tools/screenshot-result.ts index da9c33b..ab3f259 100644 --- a/src/server/tools/screenshot-result.ts +++ b/src/server/tools/screenshot-result.ts @@ -3,6 +3,7 @@ * `structuredContent` payload conforming to `screenshotOutputShape`, required * once the tool declares an `outputSchema` — the SDK (^1.29) throws `McpError` * at runtime if a non-error `CallToolResult` lacks matching `structuredContent`. + * Disk-persistence (`writeScreenshotOrError`) lives in `screenshot-write.ts`. * @module server/tools/screenshot-result */ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; @@ -16,6 +17,7 @@ export const screenshotOutputShape = { notes: z.array(z.string()).optional(), url: z.string().optional(), marks: z.number().optional(), + path: z.string().optional(), }; type Structured = z.infer>; @@ -31,36 +33,53 @@ function single(base64: string, mimeType: string, note: string, structured: Stru }; } -/** Single-element capture (`ref`). */ -export function elementScreenshotResult(base64: string, ref: number | string): CallToolResult { +/** Single-element capture (`ref`). `path`, when given, has already been written by the caller. */ +export function elementScreenshotResult(base64: string, ref: number | string, path?: string): CallToolResult { const note = `element ref=${ref}`; return single(base64, "image/png", note, { kind: "element", count: 1, mimeType: "image/png", notes: [note], + ...(path ? { path } : {}), }); } /** Full-page / default single-viewport capture. */ -export function pageScreenshotResult(base64: string, url: string): CallToolResult { +export function pageScreenshotResult(base64: string, url: string, path?: string): CallToolResult { const note = `screenshot of ${url}`; - return single(base64, "image/png", note, { kind: "page", count: 1, mimeType: "image/png", url, notes: [note] }); + return single(base64, "image/png", note, { + kind: "page", + count: 1, + mimeType: "image/png", + url, + notes: [note], + ...(path ? { path } : {}), + }); } /** Annotated capture (`annotate: true`). Kept as JPEG to match `annotatedScreenshot`. */ -export function annotatedScreenshotResult(base64: string, url: string, marks: number): CallToolResult { +export function annotatedScreenshotResult( + base64: string, + url: string, + marks: number, + path?: string, +): CallToolResult { return single(base64, "image/jpeg", JSON.stringify({ url, marks }), { kind: "annotated", count: 1, mimeType: "image/jpeg", url, marks, + ...(path ? { path } : {}), }); } /** Multi-viewport capture: several images, each preceded by a label note. */ -export function multiScreenshotResult(items: Array<{ base64: string; note: string }>): CallToolResult { +export function multiScreenshotResult( + items: Array<{ base64: string; note: string }>, + path?: string, +): CallToolResult { const content = items.flatMap((it) => [ { type: "text" as const, text: it.note }, { type: "image" as const, data: it.base64, mimeType: "image/png" as const }, @@ -72,6 +91,7 @@ export function multiScreenshotResult(items: Array<{ base64: string; note: strin count: items.length, mimeType: "image/png", notes: items.map((it) => it.note), + ...(path ? { path } : {}), }, }; } diff --git a/src/server/tools/screenshot-write.ts b/src/server/tools/screenshot-write.ts new file mode 100644 index 0000000..d9c828f --- /dev/null +++ b/src/server/tools/screenshot-write.ts @@ -0,0 +1,75 @@ +/** + * Disk-persistence helpers for `browser_screenshot`'s `path` option: validate + * the target extension against the capture's mime type, write the bytes, and + * fold that into the branch's success/error `CallToolResult`. Split out of + * `screenshot-result.ts` (and out of `screenshot.ts`'s call sites) to keep + * every screenshot file under the project's 100-line SOLID limit. + * @module server/tools/screenshot-write + */ +import { extname } from "node:path"; +import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { writeFileBytes } from "../../lib/fs.js"; +import { errorResult } from "../result.js"; + +/** Extension(s) accepted for each mime type this tool ever produces. */ +const MIME_EXTS: Record = { + "image/png": [".png"], + "image/jpeg": [".jpg", ".jpeg"], +}; + +/** + * Writes `data` to `path` on disk, gated on `path`'s extension matching + * `mimeType` — the mime is known upfront from the capture branch (never + * sniffed from bytes), so a mismatch (e.g. `.png` for the JPEG `annotate` + * branch) is rejected rather than silently renamed. This extension gate is + * stricter than `pdf.ts`, which performs no validation and accepts any path + * verbatim; the trait actually shared with `pdf.ts`/`visual-diff.ts` is + * accepting an arbitrary caller-supplied local path with no implicit + * renaming, sandboxing, or traversal check (same accepted trust model). + * Returns an error message on mismatch, else `undefined` once written. + */ +export function writeScreenshotOrError(path: string, data: Buffer, mimeType: string): string | undefined { + const valid = MIME_EXTS[mimeType] ?? []; + const ext = extname(path).toLowerCase(); + if (!valid.includes(ext)) { + return `path extension "${ext || "(none)"}" does not match ${mimeType} output (expected ${valid.join(" or ")})`; + } + writeFileBytes(path, data); + return undefined; +} + +/** + * Runs the optional `path` write for a single-image capture, then delegates + * to `onSuccess` for the branch's normal result — or short-circuits with a + * `path_extension_mismatch` error `CallToolResult` when the write is + * rejected. When `path` is `undefined`, `onSuccess` runs immediately (no-op + * write), matching the caller's original "only write when `path` is set" + * behavior. + */ +export function withOptionalWrite( + path: string | undefined, + data: Buffer, + mimeType: string, + onSuccess: () => CallToolResult, +): CallToolResult { + if (path) { + const err = writeScreenshotOrError(path, data, mimeType); + if (err) return errorResult(err, "path_extension_mismatch"); + } + return onSuccess(); +} + +/** + * Rejects `path` when the requested capture would produce more than one + * image — `browser_screenshot`'s `path` option only writes a single file, so + * multi-viewport requests combined with `path` are unsupported. Returns the + * `path_multi_viewport_unsupported` error `CallToolResult`, or `undefined` + * when `path` is compatible with `viewportCount`. + */ +export function validateMultiViewportPath(path: string | undefined, viewportCount: number): CallToolResult | undefined { + if (!path || viewportCount <= 1) return undefined; + return errorResult( + "path is not supported when viewports.length > 1 (multiple images); use a single viewport with path", + "path_multi_viewport_unsupported", + ); +} diff --git a/src/server/tools/screenshot.ts b/src/server/tools/screenshot.ts index a13831b..180e590 100644 --- a/src/server/tools/screenshot.ts +++ b/src/server/tools/screenshot.ts @@ -1,6 +1,5 @@ /** - * Screenshot tool: returns a PNG as MCP image content. Optionally targets a - * single element by `ref`, or captures across one/several viewports (responsive). + * Screenshot tool: returns a PNG as MCP image content. Optionally targets a single element by `ref`, or captures across one/several viewports (responsive). * @module server/tools/screenshot */ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; @@ -20,6 +19,7 @@ import { pageScreenshotResult, screenshotOutputShape, } from "./screenshot-result.js"; +import { validateMultiViewportPath, withOptionalWrite } from "./screenshot-write.js"; import { withSession } from "./with-session.js"; const VIEWPORT_SCHEMA = z.union([ @@ -34,7 +34,7 @@ export function registerScreenshotTool(server: McpServer, sessions: SessionManag { title: "Screenshot", description: - "Capture the live page as PNG(s) for vision. Pass `ref` for one element, `viewport` for one size, or `viewports` (e.g. [\"mobile\",\"desktop\"]) for a responsive set. Pass `colorScheme` (\"light\"|\"dark\") to capture a theme — emulates `prefers-color-scheme` AND toggles the `themeClass` (default \"dark\") on for Tailwind/shadcn class themes, then restores.", + "Capture the live page as PNG(s) for vision. Pass `ref` for one element, `viewport` for one size, or `viewports` (e.g. [\"mobile\",\"desktop\"]) for a responsive set. Pass `colorScheme` (\"light\"|\"dark\") to capture a theme — emulates `prefers-color-scheme` AND toggles the `themeClass` (default \"dark\") on for Tailwind/shadcn class themes, then restores. Pass `path` to also write the image to disk (single-image captures only — rejected when `viewports.length>1`); its extension must match the output mime (`.png`, or `.jpg`/`.jpeg` for `annotate`).", inputSchema: { sessionId: z.string(), fullPage: z.boolean().optional(), @@ -44,11 +44,13 @@ export function registerScreenshotTool(server: McpServer, sessions: SessionManag colorScheme: z.enum(["light", "dark"]).optional(), themeClass: z.string().optional(), annotate: z.boolean().optional(), + path: z.string().optional(), }, outputSchema: screenshotOutputShape, }, async (args) => { const a = args as Record; + const path = typeof a.path === "string" && a.path.length > 0 ? a.path : undefined; return withSession(sessions, String(a.sessionId), async (s) => { const scheme = a.colorScheme as "light" | "dark" | undefined; const restore = scheme @@ -56,32 +58,38 @@ export function registerScreenshotTool(server: McpServer, sessions: SessionManag : null; try { if (typeof a.ref === "number" || typeof a.ref === "string") { - const locator = refLocator(s.page, a.ref); + const ref = a.ref; + const locator = refLocator(s.page, ref); if (!locator || (await locator.count()) === 0) return errorResult("ref_not_found"); const buf = await locator.screenshot({ timeout: 5_000 }); - return elementScreenshotResult(buf.toString("base64"), a.ref); + return withOptionalWrite(path, buf, "image/png", () => elementScreenshotResult(buf.toString("base64"), ref, path)); } const fullPage = Boolean(a.fullPage); const list = (Array.isArray(a.viewports) ? a.viewports : a.viewport ? [a.viewport] : []) as ViewportInput[]; + const multiErr = validateMultiViewportPath(path, list.length); + if (multiErr) return multiErr; if (list.length === 0) { if (a.annotate === true) { await captureSnapshot(s.page); const shot = await annotatedScreenshot(s.page); - return annotatedScreenshotResult(shot.base64, s.page.url(), shot.marks); + const data = Buffer.from(shot.base64, "base64"); + return withOptionalWrite(path, data, "image/jpeg", () => annotatedScreenshotResult(shot.base64, s.page.url(), shot.marks, path)); } const buffer = await s.page.screenshot({ fullPage }); - return pageScreenshotResult(buffer.toString("base64"), s.page.url()); + return withOptionalWrite(path, buffer, "image/png", () => pageScreenshotResult(buffer.toString("base64"), s.page.url(), path)); } const original = s.page.viewportSize(); const shots: Array<{ base64: string; note: string }> = []; + let lastBuf: Buffer = Buffer.alloc(0); for (const v of list) { await s.page.setViewportSize(resolveViewport(v)); await settleForCapture(s.page); const buf = await s.page.screenshot({ fullPage, animations: "disabled" }); shots.push({ base64: buf.toString("base64"), note: viewportLabel(v) }); + lastBuf = buf; } if (original) await s.page.setViewportSize(original); - return multiScreenshotResult(shots); + return withOptionalWrite(path, lastBuf, "image/png", () => multiScreenshotResult(shots, path)); } finally { if (restore) await restore(); } diff --git a/src/server/tools/snapshot.ts b/src/server/tools/snapshot.ts index c1e3ff4..faa39a2 100644 --- a/src/server/tools/snapshot.ts +++ b/src/server/tools/snapshot.ts @@ -24,7 +24,7 @@ export function registerSnapshotTools(server: McpServer, sessions: SessionManage { title: "Snapshot", description: - "Return the indexed interactive elements of the live page, including those inside open Shadow DOM and iframes (same- and cross-origin). Use each element's `ref` (e.g. \"12\" or \"3:4\" for a sub-frame) with browser_act for deterministic targeting. Pass `selectors:true` to also get a durable CSS `selector` per element. Pass `prune:true` to drop elements hidden for accessibility (aria-hidden, display:none, or an aria-hidden/display:none ancestor, or visibility:hidden/collapse) — off by default, output unchanged. Pass `annotate:true` for a Set-of-Marks JPEG screenshot with numbered badges (= each `ref`) — for vision models: they see the page and target by ref.", + "Return the indexed interactive elements of the live page, including those inside open Shadow DOM and iframes (same- and cross-origin). Use each element's `ref` (e.g. \"12\" or \"3:4\" for a sub-frame) with browser_act for deterministic targeting. Pass `selectors:true` to also get a durable CSS `selector` per element. Pass `prune:true` to drop only genuinely hidden or decorative elements: CSS-hidden (`display:none`/`visibility:hidden`/`content-visibility`, self or ancestor, via `checkVisibility()`) OR decorative (under an `aria-hidden` ancestor AND not focusable). A visible, focusable element is KEPT even under an `aria-hidden` ancestor — so an open modal's controls stay in the snapshot. Off by default, output unchanged. Pass `annotate:true` for a Set-of-Marks JPEG screenshot with numbered badges (= each `ref`) — for vision models: they see the page and target by ref.", inputSchema: { sessionId: z.string(), selectors: z.boolean().optional(), diff --git a/tests/integration/snapshot-prune.test.ts b/tests/integration/snapshot-prune.test.ts new file mode 100644 index 0000000..1137193 --- /dev/null +++ b/tests/integration/snapshot-prune.test.ts @@ -0,0 +1,96 @@ +/** + * End-to-end coverage of `browser_snapshot`'s `prune:true` (the "C4" rule): + * an element is dropped only if it is genuinely hidden (`Element. + * checkVisibility()` false — display:none / visibility:hidden|collapse) OR it + * is decorative under an `aria-hidden` ancestor (present but NOT focusable). + * Split out of snapshot.test.ts to stay under the SOLID line cap. + * @module tests/integration/snapshot-prune + */ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { resolveConfig } from "../../src/agent/config.js"; +import { captureSnapshot } from "../../src/extraction/snapshot.js"; +import { SessionManager } from "../../src/session/manager.js"; + +// A VISIBLE + FOCUSABLE element under an `aria-hidden` ancestor — +// `HiddenAriaFocusable`, a real `" + + "" + + "" + + "" + + ""; +const PRUNE_URL = `data:text/html,${encodeURIComponent(PRUNE_PAGE)}`; + +test( + "prune:true keeps visible+focusable aria-hidden elements (modal fix), drops genuinely hidden/decorative ones", + { timeout: 120_000 }, + async () => { + const sessions = new SessionManager(); + const session = await sessions.open(resolveConfig({ headless: true, engine: "patchright" })); + try { + await session.page.goto(PRUNE_URL, { waitUntil: "domcontentloaded", timeout: 30_000 }); + + const unpruned = await captureSnapshot(session.page); + const unprunedTexts = unpruned.map((e) => e.text).sort(); + assert.deepEqual( + unprunedTexts, + ["HiddenAriaDecorative", "HiddenAriaFocusable", "HiddenDisplay", "HiddenVisibility", "Visible"], + "default (prune omitted) is byte-for-byte the pre-pruning behavior", + ); + + const pruned = await captureSnapshot(session.page, false, true); + const prunedTexts = pruned.map((e) => e.text).sort(); + assert.deepEqual( + prunedTexts, + ["HiddenAriaFocusable", "Visible"], + "prune:true keeps the visible+focusable aria-hidden button, drops the decorative/hidden ones", + ); + } finally { + await sessions.close(session.id); + } + }, +); + +// The exact bug report: a SPA marks a wrapper `aria-hidden="true"` that also +// wraps the open modal itself (not just the page behind it) — the modal's own +// interactive contents must survive prune:true, while a decoy hidden sibling +// under the same aria-hidden ancestor is still dropped. +const MODAL_PAGE = + ""; +const MODAL_URL = `data:text/html,${encodeURIComponent(MODAL_PAGE)}`; + +test( + "prune:true keeps a visible+focusable button inside an aria-hidden-wrapped open dialog (the modal bug)", + { timeout: 120_000 }, + async () => { + const sessions = new SessionManager(); + const session = await sessions.open(resolveConfig({ headless: true, engine: "patchright" })); + try { + await session.page.goto(MODAL_URL, { waitUntil: "domcontentloaded", timeout: 30_000 }); + + const pruned = await captureSnapshot(session.page, false, true); + assert.ok(pruned.length >= 1, "the dialog's interactive content should survive prune:true"); + assert.ok( + pruned.some((e) => e.text === "Action"), + "the visible+focusable dialog button must be kept even under an aria-hidden ancestor", + ); + assert.ok( + pruned.every((e) => e.text !== "GhostSibling"), + "a display:none sibling under the same aria-hidden ancestor must still be pruned", + ); + } finally { + await sessions.close(session.id); + } + }, +); diff --git a/tests/integration/snapshot.test.ts b/tests/integration/snapshot.test.ts index 0fc9be9..27fb5d8 100644 --- a/tests/integration/snapshot.test.ts +++ b/tests/integration/snapshot.test.ts @@ -78,37 +78,9 @@ test("pick types into a combobox and clicks the matching suggestion", { timeout: } }); -const PRUNE_PAGE = - "" + - "" + - ""; -const PRUNE_URL = `data:text/html,${encodeURIComponent(PRUNE_PAGE)}`; - -test( - "prune:true drops aria-hidden/display:none elements; prune omitted keeps the exact prior set", - { timeout: 120_000 }, - async () => { - const sessions = new SessionManager(); - const session = await sessions.open(resolveConfig({ headless: true, engine: "patchright" })); - try { - await session.page.goto(PRUNE_URL, { waitUntil: "domcontentloaded", timeout: 30_000 }); - - const unpruned = await captureSnapshot(session.page); - const unprunedTexts = unpruned.map((e) => e.text).sort(); - assert.deepEqual( - unprunedTexts, - ["HiddenAria", "HiddenDisplay", "Visible"], - "default (prune omitted) is byte-for-byte the pre-pruning behavior", - ); - - const pruned = await captureSnapshot(session.page, false, true); - const prunedTexts = pruned.map((e) => e.text).sort(); - assert.deepEqual(prunedTexts, ["Visible"], "prune:true drops the aria-hidden and display:none elements"); - } finally { - await sessions.close(session.id); - } - }, -); +// `prune:true` coverage (C4 rule: genuinely-hidden OR decorative-under-aria- +// hidden) moved to tests/integration/snapshot-prune.test.ts to keep this file +// under the SOLID line cap — see that file for the modal-fix regression test. test( "browser_snapshot and browser_act return structuredContent honoring their declared outputSchema (no McpError)", diff --git a/tests/unit/screenshot-tool.test.ts b/tests/unit/screenshot-tool.test.ts index 34eefee..1ac84fa 100644 --- a/tests/unit/screenshot-tool.test.ts +++ b/tests/unit/screenshot-tool.test.ts @@ -6,6 +6,9 @@ * @module tests/unit/screenshot-tool */ import { describe, expect, test } from "bun:test"; +import { existsSync, readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { z } from "zod"; @@ -94,4 +97,52 @@ describe("browser_screenshot outputSchema", () => { expect(() => outputSchema.parse(res.structuredContent)).not.toThrow(); expect(outputSchema.parse(res.structuredContent).kind).toBe("annotated"); }); + + test("full-page branch: without path, structuredContent.path stays undefined", async () => { + const { server, getHandler } = mockServer(); + registerScreenshotTool(server, fakeSessions(fakePage())); + const res = await getHandler()({ sessionId: "s", fullPage: true }); + const parsed = outputSchema.parse(res.structuredContent); + expect(parsed.path).toBeUndefined(); + }); + + test("full-page branch: with path, writes the file and returns {path}", async () => { + const out = join(tmpdir(), `fuse-screenshot-${Date.now()}.png`); + const { server, getHandler } = mockServer(); + registerScreenshotTool(server, fakeSessions(fakePage())); + const res = await getHandler()({ sessionId: "s", fullPage: true, path: out }); + const parsed = outputSchema.parse(res.structuredContent); + expect(parsed.path).toBe(out); + expect(readFileSync(out).toString()).toBe("PNG-PAGE"); + }); + + test("annotate branch: mismatched extension (.png for JPEG output) is rejected, not silently renamed", async () => { + const out = join(tmpdir(), `fuse-screenshot-mismatch-${Date.now()}.png`); + const { server, getHandler } = mockServer(); + registerScreenshotTool(server, fakeSessions(fakePage())); + const res = await getHandler()({ sessionId: "s", annotate: true, path: out }); + expect(res.isError).toBe(true); + expect((res.structuredContent as Record).code).toBe("path_extension_mismatch"); + expect(existsSync(out)).toBe(false); + }); + + test("multi-viewport branch: path with viewports.length > 1 returns an explicit error", async () => { + const out = join(tmpdir(), `fuse-screenshot-multi-${Date.now()}.png`); + const { server, getHandler } = mockServer(); + registerScreenshotTool(server, fakeSessions(fakePage())); + const res = await getHandler()({ sessionId: "s", viewports: ["mobile", "desktop"], path: out }); + expect(res.isError).toBe(true); + expect((res.structuredContent as Record).code).toBe("path_multi_viewport_unsupported"); + expect(existsSync(out)).toBe(false); + }); + + test("multi-viewport branch: path with a single-item viewports array writes the file", async () => { + const out = join(tmpdir(), `fuse-screenshot-single-multi-${Date.now()}.png`); + const { server, getHandler } = mockServer(); + registerScreenshotTool(server, fakeSessions(fakePage())); + const res = await getHandler()({ sessionId: "s", viewports: ["mobile"], path: out }); + const parsed = outputSchema.parse(res.structuredContent); + expect(parsed).toMatchObject({ kind: "multi", count: 1, path: out }); + expect(readFileSync(out).toString()).toBe("PNG-PAGE"); + }); }); diff --git a/tests/unit/snapshot-prune.test.ts b/tests/unit/snapshot-prune.test.ts index a1a3437..937bb13 100644 --- a/tests/unit/snapshot-prune.test.ts +++ b/tests/unit/snapshot-prune.test.ts @@ -1,68 +1,85 @@ /** - * Unit tests for the opt-in `prune` pruning logic: the DOM-hiding predicate - * (`isHiddenForAriaMirror`, a Node-testable mirror of the browser-side - * `isElementHiddenForAria`) and the pure keep/drop decision (`shouldKeep`). + * Unit tests for the opt-in `prune` pruning logic (the "C4" rule): the + * DOM-hiding predicate (`isPrunableMirror`, a Node-testable mirror of the + * browser-side `isPrunable`) and the pure keep/drop decision (`shouldKeep`). */ import { describe, expect, test } from "bun:test"; -import { isHiddenForAriaMirror } from "../../src/extraction/snapshot-hidden.js"; +import { isPrunableMirror } from "../../src/extraction/snapshot-hidden.js"; import { shouldKeep } from "../../src/extraction/snapshot.js"; -describe("isHiddenForAriaMirror", () => { - test("visible leaf node is not hidden", () => { - expect(isHiddenForAriaMirror({})).toBe(false); +describe("isPrunableMirror", () => { + test("visible leaf node is not prunable", () => { + expect(isPrunableMirror({})).toBe(false); }); - test("aria-hidden=true on the node itself hides it", () => { - expect(isHiddenForAriaMirror({ ariaHidden: "true" })).toBe(true); - }); - - test("aria-hidden=true on an ancestor hides it (sticky, cannot be undone)", () => { - const node = { ariaHidden: "false", parent: { ariaHidden: "true" } }; - expect(isHiddenForAriaMirror(node)).toBe(true); - }); - - test("display:none on an ancestor hides it (sticky)", () => { + test("display:none on an ancestor is prunable (CSS-hidden, sticky)", () => { const node = { parent: { parent: { display: "none" } } }; - expect(isHiddenForAriaMirror(node)).toBe(true); + expect(isPrunableMirror(node)).toBe(true); }); - test("display:contents on an ancestor does NOT hide it", () => { + test("display:contents on an ancestor is NOT prunable", () => { const node = { parent: { display: "contents" } }; - expect(isHiddenForAriaMirror(node)).toBe(false); + expect(isPrunableMirror(node)).toBe(false); }); - test("visibility:hidden on the node itself hides it", () => { - expect(isHiddenForAriaMirror({ visibility: "hidden" })).toBe(true); + test("visibility:hidden on the node itself is prunable", () => { + expect(isPrunableMirror({ visibility: "hidden" })).toBe(true); }); - test("visibility:collapse on the node itself hides it", () => { - expect(isHiddenForAriaMirror({ visibility: "collapse" })).toBe(true); + test("visibility:collapse on the node itself is prunable", () => { + expect(isPrunableMirror({ visibility: "collapse" })).toBe(true); }); - test("an ancestor's visibility:hidden does not hide this node (overridable, non-sticky)", () => { + test("an ancestor's visibility:hidden does not prune this node (overridable, non-sticky)", () => { // getComputedStyle already resolves inheritance/override; a node whose OWN // resolved visibility is "visible" (e.g. a descendant re-enabled it) must - // stay visible regardless of what an ancestor originally declared. + // stay non-prunable regardless of what an ancestor originally declared. const node = { visibility: "visible", parent: { visibility: "hidden" } }; - expect(isHiddenForAriaMirror(node)).toBe(false); + expect(isPrunableMirror(node)).toBe(false); + }); + + test("aria-hidden ancestor + NOT focusable is prunable (decorative, the pre-fix behavior)", () => { + const node = { focusable: false, parent: { ariaHidden: "true" } }; + expect(isPrunableMirror(node)).toBe(true); + }); + + test("aria-hidden ancestor + focusable is KEPT, not prunable (the modal fix)", () => { + // e.g. a