fix: handle screencast file extensions case-insensitively and clean up temp dir on failure (#2207)

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>

src/tools/screencast.ts

@@ -48,17 +48,29 @@ export const startScreencast = definePageTool(args => ({
       return;
     }
 
-    const filePath = request.params.filePath ?? (await generateTempFilePath());
-    let enforcedExtension = '.mp4' as `.${string}`;
-    let format: VideoFormat = 'mp4';
+    const requestedFilePath = request.params.filePath;
+    const filePath = requestedFilePath ?? (await generateTempFilePath());
 
-    for (const supportedExtension of supportedExtensions) {
-      if (filePath.endsWith(supportedExtension)) {
-        enforcedExtension = supportedExtension;
-        format = supportedExtension.substring(1) as VideoFormat;
-        break;
-      }
+    // Match the extension case-insensitively so e.g. `.WEBM` is recognized as
+    // WebM. An explicitly requested but unsupported extension is rejected
+    // rather than being silently rewritten to `.mp4` (which would change both
+    // the format and the output path from what was requested). A missing
+    // extension falls back to `.mp4`. The matched extension is normalized to
+    // lower case.
+    const requestedExtension = path.extname(filePath);
+    const matchedExtension = supportedExtensions.find(
+      supportedExtension =>
+        supportedExtension === requestedExtension.toLowerCase(),
+    );
+    if (!matchedExtension && requestedExtension !== '') {
+      throw new Error(
+        `Unsupported screencast file extension "${requestedExtension}". ` +
+          `Supported formats: ${supportedExtensions.join(', ')} (case-insensitive).`,
+      );
     }
+    const enforcedExtension: `.${string}` = matchedExtension ?? '.mp4';
+    const format: VideoFormat = (matchedExtension?.substring(1) ??
+      'mp4') as VideoFormat;
 
     const resolvedPath = ensureExtension(
       path.resolve(filePath),
@@ -75,6 +87,18 @@ export const startScreencast = definePageTool(args => ({
         ffmpegPath: args?.experimentalFfmpegPath,
       });
     } catch (err) {
+      // If we generated a temporary directory for this recording, remove it so
+      // a failed start (e.g. ffmpeg missing) does not leak an empty directory.
+      if (requestedFilePath === undefined) {
+        try {
+          await fs.rm(path.dirname(resolvedPath), {
+            recursive: true,
+            force: true,
+          });
+        } catch {
+          // no-op
+        }
+      }
       const message = err instanceof Error ? err.message : String(err);
       if (message.includes('ENOENT') && message.includes('ffmpeg')) {
         throw new Error(

tests/tools/screencast.test.ts

@@ -5,6 +5,8 @@
  */
 
 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';
@@ -56,6 +58,53 @@ describe('screencast', () => {
       });
     });
 
+    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();
@@ -126,6 +175,30 @@ describe('screencast', () => {
       });
     });
 
+    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();