chromedevtools--chrome-devtools-mcp
ba80096521
Fixes #2206 ### Problem `screencast_start` matched the requested file extension with a **case-sensitive** `endsWith()` against `['.webm', '.mp4']` and **silently fell back to `.mp4`** when nothing matched. Combined with `ensureExtension()` (which replaces the extension), a request for `demo.WEBM` was recorded as **MP4** to **`demo.mp4`** — a different format *and* path than requested — and any unsupported extension (e.g. `recording.avi`) silently became `.mp4`. Separately, when `screencast_start` is called without a `filePath`, it creates a temp directory via `mkdtemp()`. If `page.screencast()` then throws (e.g. ffmpeg missing), that directory was leaked. ### Changes Two commits: 1. **`fix: match screencast extension case-insensitively and reject unsupported ones`** — match via `path.extname().toLowerCase()`; reject an explicitly requested but unsupported extension with an explicit error listing the supported formats; a missing extension still defaults to `.mp4`. 2. **`fix: clean up screencast temp directory when recording fails to start`** — remove the generated temp dir in the `catch` handler, but only when we own the generated path (never when the caller supplied `filePath`). | requested | before | after | | --------------- | --------------- | -------------- | | `demo.WEBM` | mp4 → `demo.mp4`| webm → `demo.webm` | | `recording.avi` | mp4 → `recording.mp4` | error (rejected) | | `demo.webm` | webm → `demo.webm` | unchanged | | *(no filePath)* | mp4 temp | unchanged | The matched extension is normalized to lower case (`demo.WEBM` → `demo.webm`). ### Testing Added three regression tests to `tests/tools/screencast.test.ts` using the existing `sinon`/`withMcpContext` harness. Verified locally against Chrome for Testing 149 (`PUPPETEER_EXECUTABLE_PATH`): - With the fix reverted, the two extension tests fail (uppercase `.WEBM` → mp4, `.avi` not rejected) and the cleanup test fails (temp dir left behind) — i.e. they fail for the right reason. - With the fix applied, the full `screencast.test.ts` suite passes (11/11). - `tsc --noEmit` and `npm run check-format` (eslint + prettier) are clean. > Note: I ran the `screencast` test file (which stubs `page.screencast`) plus typecheck/lint locally; the rest of the browser-based suite I left to CI. ### Notes for reviewers - I chose to **`throw`** for an unsupported explicit extension (consistent with the ffmpeg-missing `throw` in the same handler and with the issue's "reject with an explicit error"). Happy to switch to the softer `appendResponseLine(...) + return` style used by the in-progress guard if you'd prefer. - The two commits are independent and can be split if you'd rather take them separately. - I left the pre-existing `as \`${string}.webm\`` assertion on `resolvedPath` untouched to keep the diff focused, though it's slightly misleading now that the default is `.mp4`. --------- Co-authored-by: Nicholas Roscino <nroscino@google.com>
291 行
9.2 KiB
TypeScript
291 行
9.2 KiB
TypeScript
/**
|
|
* @license
|
|
* Copyright 2026 Google LLC
|
|
* SPDX-License-Identifier: Apache-2.0
|
|
*/
|
|
|
|
import assert from 'node:assert';
|
|
import fs from 'node:fs/promises';
|
|
import path from 'node:path';
|
|
import {describe, it, afterEach} from 'node:test';
|
|
|
|
import sinon from 'sinon';
|
|
|
|
import type {ParsedArguments} from '../../src/bin/chrome-devtools-mcp-cli-options.js';
|
|
import {startScreencast, stopScreencast} from '../../src/tools/screencast.js';
|
|
import {withMcpContext} from '../utils.js';
|
|
|
|
function createMockRecorder() {
|
|
return {
|
|
stop: sinon.stub().resolves(),
|
|
};
|
|
}
|
|
|
|
describe('screencast', () => {
|
|
afterEach(() => {
|
|
sinon.restore();
|
|
});
|
|
|
|
describe('screencast_start', () => {
|
|
it('starts a screencast recording with filePath', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const mockRecorder = createMockRecorder();
|
|
const selectedPage = context.getSelectedPptrPage();
|
|
const screencastStub = sinon
|
|
.stub(selectedPage, 'screencast')
|
|
.resolves(mockRecorder as never);
|
|
|
|
await startScreencast().handler(
|
|
{
|
|
params: {filePath: '/tmp/test-recording.mp4'},
|
|
page: context.getSelectedMcpPage(),
|
|
},
|
|
response,
|
|
context,
|
|
);
|
|
|
|
sinon.assert.calledOnce(screencastStub);
|
|
const callArgs = screencastStub.firstCall.args[0];
|
|
assert.ok(callArgs);
|
|
assert.ok(callArgs.path?.endsWith('test-recording.mp4'));
|
|
|
|
assert.ok(context.getScreenRecorder() !== null);
|
|
assert.ok(
|
|
response.responseLines
|
|
.join('\n')
|
|
.includes('Screencast recording started'),
|
|
);
|
|
});
|
|
});
|
|
|
|
it('records WebM for an uppercase extension (case-insensitive)', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const mockRecorder = createMockRecorder();
|
|
const selectedPage = context.getSelectedPptrPage();
|
|
const screencastStub = sinon
|
|
.stub(selectedPage, 'screencast')
|
|
.resolves(mockRecorder as never);
|
|
|
|
await startScreencast().handler(
|
|
{
|
|
params: {filePath: '/tmp/test-recording.WEBM'},
|
|
page: context.getSelectedMcpPage(),
|
|
},
|
|
response,
|
|
context,
|
|
);
|
|
|
|
sinon.assert.calledOnce(screencastStub);
|
|
const callArgs = screencastStub.firstCall.args[0];
|
|
assert.ok(callArgs);
|
|
assert.strictEqual(callArgs.format, 'webm');
|
|
assert.ok(callArgs.path?.endsWith('.webm'));
|
|
});
|
|
});
|
|
|
|
it('rejects an unsupported extension instead of silently using mp4', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const selectedPage = context.getSelectedPptrPage();
|
|
const screencastStub = sinon.stub(selectedPage, 'screencast');
|
|
|
|
await assert.rejects(
|
|
startScreencast().handler(
|
|
{
|
|
params: {filePath: '/tmp/recording.avi'},
|
|
page: context.getSelectedMcpPage(),
|
|
},
|
|
response,
|
|
context,
|
|
),
|
|
/Unsupported screencast file extension/,
|
|
);
|
|
|
|
sinon.assert.notCalled(screencastStub);
|
|
assert.strictEqual(context.getScreenRecorder(), null);
|
|
});
|
|
});
|
|
|
|
it('starts a screencast recording with temp file when no filePath', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const mockRecorder = createMockRecorder();
|
|
const selectedPage = context.getSelectedPptrPage();
|
|
const screencastStub = sinon
|
|
.stub(selectedPage, 'screencast')
|
|
.resolves(mockRecorder as never);
|
|
|
|
await startScreencast().handler(
|
|
{params: {}, page: context.getSelectedMcpPage()},
|
|
response,
|
|
context,
|
|
);
|
|
|
|
sinon.assert.calledOnce(screencastStub);
|
|
const callArgs = screencastStub.firstCall.args[0];
|
|
assert.ok(callArgs);
|
|
assert.ok(callArgs.path?.endsWith('.mp4'));
|
|
assert.ok(context.getScreenRecorder() !== null);
|
|
});
|
|
});
|
|
|
|
it('errors if a recording is already active', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const mockRecorder = createMockRecorder();
|
|
context.setScreenRecorder({
|
|
recorder: mockRecorder as never,
|
|
filePath: '/tmp/existing.mp4',
|
|
});
|
|
|
|
const selectedPage = context.getSelectedPptrPage();
|
|
const screencastStub = sinon.stub(selectedPage, 'screencast');
|
|
|
|
await startScreencast().handler(
|
|
{params: {}, page: context.getSelectedMcpPage()},
|
|
response,
|
|
context,
|
|
);
|
|
|
|
sinon.assert.notCalled(screencastStub);
|
|
assert.ok(
|
|
response.responseLines
|
|
.join('\n')
|
|
.includes('a screencast recording is already in progress'),
|
|
);
|
|
});
|
|
});
|
|
|
|
it('provides a clear error when ffmpeg is not found', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const selectedPage = context.getSelectedPptrPage();
|
|
const error = new Error('spawn ffmpeg ENOENT');
|
|
sinon.stub(selectedPage, 'screencast').rejects(error);
|
|
|
|
await assert.rejects(
|
|
startScreencast().handler(
|
|
{
|
|
params: {filePath: '/tmp/test.mp4'},
|
|
page: context.getSelectedMcpPage(),
|
|
},
|
|
response,
|
|
context,
|
|
),
|
|
/ffmpeg is required for screencast recording/,
|
|
);
|
|
|
|
assert.strictEqual(context.getScreenRecorder(), null);
|
|
});
|
|
});
|
|
|
|
it('cleans up the generated temp directory if recording fails to start', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const selectedPage = context.getSelectedPptrPage();
|
|
const screencastStub = sinon
|
|
.stub(selectedPage, 'screencast')
|
|
.rejects(new Error('spawn ffmpeg ENOENT'));
|
|
|
|
await assert.rejects(
|
|
startScreencast().handler(
|
|
{params: {}, page: context.getSelectedMcpPage()},
|
|
response,
|
|
context,
|
|
),
|
|
/ffmpeg is required for screencast recording/,
|
|
);
|
|
|
|
// The temp directory generateTempFilePath() created must be removed.
|
|
const tempPath = screencastStub.firstCall.args[0]?.path as string;
|
|
assert.ok(tempPath);
|
|
await assert.rejects(fs.access(path.dirname(tempPath)));
|
|
assert.strictEqual(context.getScreenRecorder(), null);
|
|
});
|
|
});
|
|
|
|
it('passes ffmpegPath from args to puppeteer', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const mockRecorder = createMockRecorder();
|
|
const selectedPage = context.getSelectedPptrPage();
|
|
const screencastStub = sinon
|
|
.stub(selectedPage, 'screencast')
|
|
.resolves(mockRecorder as never);
|
|
|
|
const experimentalFfmpegPath = '/custom/path/to/ffmpeg';
|
|
await startScreencast({
|
|
experimentalFfmpegPath,
|
|
} as ParsedArguments).handler(
|
|
{params: {}, page: context.getSelectedMcpPage()},
|
|
response,
|
|
context,
|
|
);
|
|
|
|
sinon.assert.calledOnce(screencastStub);
|
|
const callArgs = screencastStub.firstCall.args[0];
|
|
assert.strictEqual(callArgs?.ffmpegPath, experimentalFfmpegPath);
|
|
});
|
|
});
|
|
});
|
|
|
|
describe('screencast_stop', () => {
|
|
it('returns an error message if no recording is active', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
assert.strictEqual(context.getScreenRecorder(), null);
|
|
await stopScreencast.handler(
|
|
{params: {}, page: context.getSelectedMcpPage()},
|
|
response,
|
|
context,
|
|
);
|
|
assert.ok(
|
|
response.responseLines
|
|
.join('\n')
|
|
.includes('no active screencast recording to stop'),
|
|
);
|
|
});
|
|
});
|
|
|
|
it('stops an active recording and reports the file path', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const mockRecorder = createMockRecorder();
|
|
const filePath = '/tmp/test-recording.mp4';
|
|
context.setScreenRecorder({
|
|
recorder: mockRecorder as never,
|
|
filePath,
|
|
});
|
|
|
|
await stopScreencast.handler(
|
|
{params: {}, page: context.getSelectedMcpPage()},
|
|
response,
|
|
context,
|
|
);
|
|
|
|
sinon.assert.calledOnce(mockRecorder.stop);
|
|
assert.strictEqual(context.getScreenRecorder(), null);
|
|
assert.ok(
|
|
response.responseLines
|
|
.join('\n')
|
|
.includes('stopped and saved to /tmp/test-recording.mp4'),
|
|
);
|
|
});
|
|
});
|
|
|
|
it('clears the recorder even if stop() throws', async () => {
|
|
await withMcpContext(async (response, context) => {
|
|
const mockRecorder = createMockRecorder();
|
|
mockRecorder.stop.rejects(new Error('ffmpeg process error'));
|
|
context.setScreenRecorder({
|
|
recorder: mockRecorder as never,
|
|
filePath: '/tmp/test.mp4',
|
|
});
|
|
|
|
await assert.rejects(
|
|
stopScreencast.handler(
|
|
{params: {}, page: context.getSelectedMcpPage()},
|
|
response,
|
|
context,
|
|
),
|
|
/ffmpeg process error/,
|
|
);
|
|
|
|
assert.strictEqual(context.getScreenRecorder(), null);
|
|
});
|
|
});
|
|
});
|
|
});
|