diff --git a/CHANGELOG.md b/CHANGELOG.md index 38365c8..a433188 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add Hermes CDP tools (`hermes_cdp`, `hermes_targets`) for inspecting the React Native Hermes JS runtime via Metro +### Changed + +- Make screenshot base64 encoding optional — `DeviceBackend.screenshot(outputPath, { encode })` accepts `{ encode: false }` to return only the file `path` and skip base64 encoding. `ScreenshotResult.data` is now optional; the default (`encode: true`) preserves existing behavior +- The Appium backend now saves the screenshot to `outputPath` when one is provided (previously the argument was ignored) + +### Fixed + +- Fix `device_screenshot` failing on macOS — the captured PNG is now read in-process via `fs.readFile(path, 'base64')` instead of shelling out to the `base64` CLI, which is not portable (BSD `base64` on macOS rejects the GNU positional-file syntax that was used previously) + ## [0.2.0] ### Added diff --git a/src/backends/adb-backend.test.ts b/src/backends/adb-backend.test.ts index 0a5e460..fe28bb5 100644 --- a/src/backends/adb-backend.test.ts +++ b/src/backends/adb-backend.test.ts @@ -1,3 +1,4 @@ +import { readFile } from 'node:fs/promises'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { @@ -8,6 +9,10 @@ import { import { findElement } from '../utils/element.js'; import * as execModule from '../utils/exec.js'; +vi.mock('node:fs/promises', () => ({ + readFile: vi.fn(), +})); + vi.mock('../utils/exec.js', () => ({ exec: vi.fn(), execStrict: vi.fn(), @@ -15,6 +20,7 @@ vi.mock('../utils/exec.js', () => ({ })); const mockExecStrict = vi.mocked(execModule.execStrict); +const mockReadFile = vi.mocked(readFile); const SAMPLE_UIAUTOMATOR_XML = ` @@ -170,3 +176,68 @@ describe('AdbBackend.getElementText', () => { ).rejects.toThrow('Element not found'); }); }); + +describe('AdbBackend.screenshot', () => { + let backend: AdbBackend; + + beforeEach(() => { + vi.clearAllMocks(); + backend = new AdbBackend('emulator-5554'); + }); + + it('captures with screencap and encodes base64 in-process by default', async () => { + mockExecStrict.mockResolvedValue(''); + mockReadFile.mockResolvedValue('QUJDREVG'); + + const result = await backend.screenshot('/tmp/a.png'); + + expect(mockExecStrict).toHaveBeenCalledWith('adb', [ + '-s', + 'emulator-5554', + 'pull', + '/sdcard/screenshot.png', + '/tmp/a.png', + ]); + expect(mockReadFile).toHaveBeenCalledWith('/tmp/a.png', 'base64'); + expect(result).toStrictEqual({ + data: 'QUJDREVG', + format: 'png', + path: '/tmp/a.png', + }); + }); + + it('never shells out to the non-portable base64 binary', async () => { + mockExecStrict.mockResolvedValue(''); + mockReadFile.mockResolvedValue('Zm9v'); + + await backend.screenshot('/tmp/a.png'); + + expect(mockExecStrict).not.toHaveBeenCalledWith( + 'base64', + expect.anything(), + ); + }); + + it('skips base64 encoding when encode is false', async () => { + mockExecStrict.mockResolvedValue(''); + + const result = await backend.screenshot('/tmp/a.png', { encode: false }); + + expect(mockReadFile).not.toHaveBeenCalled(); + expect(result).toStrictEqual({ + data: undefined, + format: 'png', + path: '/tmp/a.png', + }); + }); + + it('defaults to a tmp path when none is provided', async () => { + mockExecStrict.mockResolvedValue(''); + mockReadFile.mockResolvedValue('YmFy'); + + const result = await backend.screenshot(); + + expect(result.path).toMatch(/^\/tmp\/device-mcp-screenshot-\d+\.png$/u); + expect(result.data).toBe('YmFy'); + }); +}); diff --git a/src/backends/adb-backend.ts b/src/backends/adb-backend.ts index 1a3907c..03df98a 100644 --- a/src/backends/adb-backend.ts +++ b/src/backends/adb-backend.ts @@ -1,5 +1,6 @@ import { spawn } from 'node:child_process'; import type { ChildProcess } from 'node:child_process'; +import { readFile } from 'node:fs/promises'; import type { DeviceBackend, @@ -7,6 +8,8 @@ import type { DeviceInfo, SnapshotResult, ScreenshotResult, + ScreenshotFileResult, + ScreenshotOptions, LogsResult, TapResult, AppStateResult, @@ -216,7 +219,25 @@ export class AdbBackend implements DeviceBackend { return xml; } - async screenshot(outputPath?: string): Promise { + screenshot( + outputPath?: string, + options?: { encode?: true }, + ): Promise; + + screenshot( + outputPath: string | undefined, + options: { encode: false }, + ): Promise; + + screenshot( + outputPath?: string, + options?: ScreenshotOptions, + ): Promise; + + async screenshot( + outputPath?: string, + options?: ScreenshotOptions, + ): Promise { const localPath = outputPath ?? `/tmp/device-mcp-screenshot-${Date.now()}.png`; await this.#adb(['shell', 'screencap', '-p', '/sdcard/screenshot.png']); @@ -228,8 +249,11 @@ export class AdbBackend implements DeviceBackend { localPath, ]); await this.#adb(['shell', 'rm', '-f', '/sdcard/screenshot.png']); - const data = await execStrict('base64', [localPath]); - return { data: data.trim(), format: 'png', path: localPath }; + if (options?.encode === false) { + return { data: undefined, format: 'png', path: localPath }; + } + const data = await readFile(localPath, 'base64'); + return { data, format: 'png', path: localPath }; } async openApp(bundleId: string): Promise { diff --git a/src/backends/appium-backend.test.ts b/src/backends/appium-backend.test.ts index 68564d0..f8c89a5 100644 --- a/src/backends/appium-backend.test.ts +++ b/src/backends/appium-backend.test.ts @@ -1,9 +1,30 @@ -import { describe, it, expect } from 'vitest'; +import { writeFile } from 'node:fs/promises'; +import { resolve as resolvePath } from 'node:path'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { + AppiumBackend, parseAppiumAndroidHierarchy, parseAppiumIosHierarchy, } from './appium-backend.js'; +import type { AttachSessionConfig } from './session-file.js'; + +const mockGetStatus = vi.fn(); +const mockExecute = vi.fn(); + +vi.mock('./webdriver-client.js', () => ({ + WebDriverClient: vi.fn().mockImplementation(() => ({ + getStatus: mockGetStatus, + execute: mockExecute, + })), + createSession: vi.fn(), +})); + +vi.mock('node:fs/promises', () => ({ + writeFile: vi.fn(), +})); + +const mockWriteFile = vi.mocked(writeFile); const SAMPLE_ANDROID_XML = ` @@ -103,3 +124,79 @@ describe('parseAppiumIosHierarchy', () => { expect(parseAppiumIosHierarchy('')).toStrictEqual([]); }); }); + +describe('AppiumBackend.screenshot', () => { + const config: AttachSessionConfig = { + mode: 'attach', + appiumUrl: 'http://localhost:4723', + sessionId: 'session-1', + platform: 'ios', + }; + + let backend: AppiumBackend; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetStatus.mockResolvedValue({}); + backend = new AppiumBackend(config); + await backend.ensureConnected(); + }); + + it('returns base64 image data from the WebDriver session by default', async () => { + mockExecute.mockResolvedValue('YmFzZTY0LWltYWdl'); + + const result = await backend.screenshot(); + + expect(mockExecute).toHaveBeenCalledWith('mobile: getScreenshot', []); + expect(mockWriteFile).not.toHaveBeenCalled(); + expect(result).toStrictEqual({ + data: 'YmFzZTY0LWltYWdl', + format: 'png', + path: undefined, + }); + }); + + it('writes to a temp file and omits base64 data when encode is false', async () => { + mockExecute.mockResolvedValue('YmFzZTY0LWltYWdl'); + + const result = await backend.screenshot(undefined, { encode: false }); + + expect(result.data).toBeUndefined(); + expect(result.format).toBe('png'); + expect(result.path).toMatch(/^\/tmp\/device-mcp-screenshot-\d+\.png$/u); + expect(mockWriteFile).toHaveBeenCalledWith( + result.path, + Buffer.from('YmFzZTY0LWltYWdl', 'base64'), + ); + }); + + it('writes the decoded image to outputPath when provided', async () => { + mockExecute.mockResolvedValue('YmFzZTY0LWltYWdl'); + + const result = await backend.screenshot('/tmp/appium-shot.png'); + + expect(mockWriteFile).toHaveBeenCalledWith( + resolvePath('/tmp/appium-shot.png'), + Buffer.from('YmFzZTY0LWltYWdl', 'base64'), + ); + expect(result.path).toBe(resolvePath('/tmp/appium-shot.png')); + }); + + it('writes to outputPath and omits base64 data when encode is false', async () => { + mockExecute.mockResolvedValue('YmFzZTY0LWltYWdl'); + + const result = await backend.screenshot('/tmp/appium-shot.png', { + encode: false, + }); + + expect(mockWriteFile).toHaveBeenCalledWith( + resolvePath('/tmp/appium-shot.png'), + Buffer.from('YmFzZTY0LWltYWdl', 'base64'), + ); + expect(result).toStrictEqual({ + data: undefined, + format: 'png', + path: resolvePath('/tmp/appium-shot.png'), + }); + }); +}); diff --git a/src/backends/appium-backend.ts b/src/backends/appium-backend.ts index f34fb98..712e73c 100644 --- a/src/backends/appium-backend.ts +++ b/src/backends/appium-backend.ts @@ -1,4 +1,5 @@ import { writeFileSync } from 'node:fs'; +import { writeFile } from 'node:fs/promises'; import { resolve as resolvePath } from 'node:path'; import type { SessionConfig } from './session-file.js'; @@ -8,6 +9,8 @@ import type { DeviceInfo, SnapshotResult, ScreenshotResult, + ScreenshotFileResult, + ScreenshotOptions, LogsResult, TapResult, AppStateResult, @@ -249,10 +252,40 @@ export class AppiumBackend implements DeviceBackend { } } - async screenshot(): Promise { + screenshot( + outputPath?: string, + options?: { encode?: true }, + ): Promise; + + screenshot( + outputPath: string | undefined, + options: { encode: false }, + ): Promise; + + screenshot( + outputPath?: string, + options?: ScreenshotOptions, + ): Promise; + + async screenshot( + outputPath?: string, + options?: ScreenshotOptions, + ): Promise { const client = this.#requireClient(); const data = await client.execute('mobile: getScreenshot', []); - return { data, format: 'png' }; + if (options?.encode === false) { + const path = resolvePath( + outputPath ?? `/tmp/device-mcp-screenshot-${Date.now()}.png`, + ); + await writeFile(path, Buffer.from(data, 'base64')); + return { data: undefined, format: 'png', path }; + } + let path: string | undefined; + if (outputPath) { + path = resolvePath(outputPath); + await writeFile(path, Buffer.from(data, 'base64')); + } + return { data, format: 'png', path }; } async openApp(bundleId: string): Promise { diff --git a/src/backends/idb-backend.test.ts b/src/backends/idb-backend.test.ts index fa41a39..75127eb 100644 --- a/src/backends/idb-backend.test.ts +++ b/src/backends/idb-backend.test.ts @@ -1,8 +1,13 @@ +import { readFile } from 'node:fs/promises'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { parseIdbHierarchy, mapIdbElement, IdbBackend } from './idb-backend.js'; import * as execModule from '../utils/exec.js'; +vi.mock('node:fs/promises', () => ({ + readFile: vi.fn(), +})); + vi.mock('../utils/exec.js', () => ({ exec: vi.fn(), execStrict: vi.fn(), @@ -18,6 +23,7 @@ vi.mock('../utils/platform.js', () => ({ const mockExec = vi.mocked(execModule.exec); const mockExecStrict = vi.mocked(execModule.execStrict); +const mockReadFile = vi.mocked(readFile); describe('parseIdbHierarchy', () => { it('parses a JSON array of elements', () => { @@ -332,3 +338,69 @@ describe('IdbBackend simctl fallback', () => { }); }); }); + +describe('IdbBackend.screenshot', () => { + const udid = 'AAAA1111-BBBB-CCCC-DDDD-EEEE2222FFFF'; + let backend: IdbBackend; + + beforeEach(() => { + vi.clearAllMocks(); + mockExec.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }); + backend = new IdbBackend(udid); + }); + + it('captures with idb and encodes base64 in-process by default', async () => { + mockExecStrict.mockResolvedValue(''); + mockReadFile.mockResolvedValue('ZmFrZS1wbmc='); + + const result = await backend.screenshot('/tmp/shot.png'); + + expect(mockExecStrict).toHaveBeenCalledWith('/usr/local/bin/idb', [ + 'screenshot', + '/tmp/shot.png', + '--udid', + udid, + ]); + expect(mockReadFile).toHaveBeenCalledWith('/tmp/shot.png', 'base64'); + expect(result).toStrictEqual({ + data: 'ZmFrZS1wbmc=', + format: 'png', + path: '/tmp/shot.png', + }); + }); + + it('never shells out to the non-portable base64 binary', async () => { + mockExecStrict.mockResolvedValue(''); + mockReadFile.mockResolvedValue('Zm9v'); + + await backend.screenshot('/tmp/shot.png'); + + expect(mockExecStrict).not.toHaveBeenCalledWith( + 'base64', + expect.anything(), + ); + }); + + it('skips base64 encoding when encode is false', async () => { + mockExecStrict.mockResolvedValue(''); + + const result = await backend.screenshot('/tmp/shot.png', { encode: false }); + + expect(mockReadFile).not.toHaveBeenCalled(); + expect(result).toStrictEqual({ + data: undefined, + format: 'png', + path: '/tmp/shot.png', + }); + }); + + it('defaults to a tmp path when none is provided', async () => { + mockExecStrict.mockResolvedValue(''); + mockReadFile.mockResolvedValue('YmFy'); + + const result = await backend.screenshot(); + + expect(result.path).toMatch(/^\/tmp\/device-mcp-screenshot-\d+\.png$/u); + expect(result.data).toBe('YmFy'); + }); +}); diff --git a/src/backends/idb-backend.ts b/src/backends/idb-backend.ts index 2ce2d84..d0686d5 100644 --- a/src/backends/idb-backend.ts +++ b/src/backends/idb-backend.ts @@ -1,5 +1,6 @@ import { execFile as execFileCb, spawn } from 'node:child_process'; import type { ChildProcess } from 'node:child_process'; +import { readFile } from 'node:fs/promises'; import type { DeviceBackend, @@ -7,6 +8,8 @@ import type { DeviceInfo, SnapshotResult, ScreenshotResult, + ScreenshotFileResult, + ScreenshotOptions, LogsResult, TapResult, AppStateResult, @@ -231,12 +234,33 @@ export class IdbBackend implements DeviceBackend { return { bundleId, state: 'Not Installed' }; } - async screenshot(outputPath?: string): Promise { + screenshot( + outputPath?: string, + options?: { encode?: true }, + ): Promise; + + screenshot( + outputPath: string | undefined, + options: { encode: false }, + ): Promise; + + screenshot( + outputPath?: string, + options?: ScreenshotOptions, + ): Promise; + + async screenshot( + outputPath?: string, + options?: ScreenshotOptions, + ): Promise { await this.ensureConnected(); const path = outputPath ?? `/tmp/device-mcp-screenshot-${Date.now()}.png`; await execStrict(this.#idbPath, ['screenshot', path, '--udid', this.#udid]); - const data = await execStrict('base64', [path]); - return { data: data.trim(), format: 'png', path }; + if (options?.encode === false) { + return { data: undefined, format: 'png', path }; + } + const data = await readFile(path, 'base64'); + return { data, format: 'png', path }; } async openApp(bundleId: string): Promise { diff --git a/src/backends/index.ts b/src/backends/index.ts index d46d56c..eab25d7 100644 --- a/src/backends/index.ts +++ b/src/backends/index.ts @@ -8,6 +8,8 @@ import type { DeviceInfo, LogsResult, ScreenshotResult, + ScreenshotFileResult, + ScreenshotOptions, SnapshotResult, TapResult, AppStateResult, @@ -97,6 +99,25 @@ export function createLazyBackend( return connecting; } + function screenshot( + outputPath?: string, + options?: { encode?: true }, + ): Promise; + function screenshot( + outputPath: string | undefined, + options: { encode: false }, + ): Promise; + function screenshot( + outputPath?: string, + options?: ScreenshotOptions, + ): Promise; + async function screenshot( + outputPath?: string, + options?: ScreenshotOptions, + ): Promise { + return (await resolve()).screenshot(outputPath, options); + } + return { get platform(): Platform { return inner?.platform ?? 'ios'; @@ -156,9 +177,7 @@ export function createLazyBackend( return (await resolve()).getAppState(bundleId); }, - async screenshot(outputPath?: string): Promise { - return (await resolve()).screenshot(outputPath); - }, + screenshot, async openApp(bundleId: string): Promise { return (await resolve()).openApp(bundleId); diff --git a/src/backends/types.ts b/src/backends/types.ts index 0dbe068..2095e37 100644 --- a/src/backends/types.ts +++ b/src/backends/types.ts @@ -39,11 +39,25 @@ export type AppStateResult = { }; export type ScreenshotResult = { + /** Base64-encoded image data. */ data: string; format: 'png' | 'jpeg'; + /** File path the screenshot was written to, when captured to disk. */ path?: string; }; +/** Result of a screenshot captured with `encode: false`: file `path` only, no base64 `data`. */ +export type ScreenshotFileResult = { + data?: undefined; + format: 'png' | 'jpeg'; + path: string; +}; + +export type ScreenshotOptions = { + /** Include base64 image data in the result. Defaults to `true`. */ + encode?: boolean; +}; + export type LogEntry = { timestamp: string; level: string; @@ -97,7 +111,18 @@ export type DeviceBackend = { getAppState(bundleId: string): Promise; - screenshot(outputPath?: string): Promise; + screenshot( + outputPath?: string, + options?: { encode?: true }, + ): Promise; + screenshot( + outputPath: string | undefined, + options: { encode: false }, + ): Promise; + screenshot( + outputPath?: string, + options?: ScreenshotOptions, + ): Promise; openApp(bundleId: string): Promise; diff --git a/src/index.ts b/src/index.ts index 65166f6..5d2ff15 100644 --- a/src/index.ts +++ b/src/index.ts @@ -13,6 +13,8 @@ export type { ElementQuery, SnapshotResult, ScreenshotResult, + ScreenshotFileResult, + ScreenshotOptions, LogsResult, TapResult, AppStateResult, diff --git a/src/tools/screenshot.ts b/src/tools/screenshot.ts index 94a3542..fbc710a 100644 --- a/src/tools/screenshot.ts +++ b/src/tools/screenshot.ts @@ -25,14 +25,19 @@ export function registerScreenshotTool( }, async ({ outputPath }) => { try { - const result = await backend.screenshot(outputPath); + const result = await backend.screenshot(outputPath, { encode: true }); + const imageContent = result.data + ? [ + { + type: 'image' as const, + data: result.data, + mimeType: `image/${result.format}`, + }, + ] + : []; return { content: [ - { - type: 'image' as const, - data: result.data, - mimeType: `image/${result.format}`, - }, + ...imageContent, { type: 'text' as const, text: result.path diff --git a/vitest.config.mts b/vitest.config.mts index 6d57ef1..363b942 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -12,10 +12,10 @@ export default defineConfig({ exclude: ['src/**/*.test.ts', 'src/**/*.test-d.ts'], thresholds: { autoUpdate: !process.env.CI, - branches: 40.31, - functions: 44.56, - lines: 38.75, - statements: 38.88, + branches: 52.38, + functions: 55.45, + lines: 52.42, + statements: 52.32, }, },