fix(screenshot): dispose element handle after take_screenshot (#2422)

Fixes #2421

## Problem

When `take_screenshot` is called with a `uid`, the handler resolves an
element handle via `getElementByUid` but never disposes it.
`getElementByUid` mints a fresh `ElementHandle` per call
(`McpPage.getElementByUid` → `node.elementHandle()`), so each element
screenshot leaks a remote-object reference for the life of the page's
execution context. This diverges from every uid-based handler in
`input.ts`, which all dispose their handles in a `finally`.

## Fix

Wrap the clip computation and the capture in `try/finally` and dispose
the handle in the `finally`, so it is also released when `getSourceBox`
or the capture throws. The capture branches themselves are unchanged —
this is a pure control-flow restructuring plus the disposal.

## Testing

- Added two regression tests: `disposes the element handle after an
element screenshot` (success path) and `disposes the element handle when
the capture fails` (capture stubbed to reject). Both fail on `main`
(`dispose` called 0 times) and pass with this change.
- `npm run test tests/tools/screenshot.test.ts` — all 21 tests pass.
- ESLint + Prettier clean on both changed files.

src/tools/screenshot.ts

@@ -156,50 +156,57 @@ export const screenshot = definePageTool(args => {
           : (request.params.quality ?? screenshotQuality);
       const fullPage = request.params.fullPage ?? false;
 
-      // Compute a downscale clip when --screenshot-max-width or
-      // --screenshot-max-height is set and the source exceeds either bound.
-      // The smaller scale factor wins so both bounds are respected while
-      // preserving aspect ratio.
-      let clip: ScreenshotClip | undefined;
-      if (
-        screenshotMaxWidth !== undefined ||
-        screenshotMaxHeight !== undefined
-      ) {
-        const box = await getSourceBox(page, element, fullPage);
-        if (box) {
-          clip = computeDownscaleClip(
-            box,
-            screenshotMaxWidth,
-            screenshotMaxHeight,
-          );
-        }
-      }
-
+      // `getElementByUid` mints a fresh ElementHandle per call, so dispose it
+      // once the capture is done to avoid leaking a remote object for the life
+      // of the page's execution context.
       let screenshot: Uint8Array;
-      if (clip) {
-        // page.screenshot with clip lets the CDP scale param downscale the
-        // capture for viewport, full-page and element shots alike. We rely on
-        // Puppeteer's default of captureBeyondViewport=true when a clip is
-        // present so element/full-page captures below the fold still work.
-        screenshot = await page.screenshot({
-          type: format,
-          quality,
-          optimizeForSpeed: true,
-          clip,
-        });
-      } else if (element) {
-        screenshot = await element.screenshot({
-          type: format,
-          quality,
-          optimizeForSpeed: true,
-        });
-      } else {
-        screenshot = await page.screenshot({
-          type: format,
-          fullPage,
-          quality,
-          optimizeForSpeed: true,
-        });
+      try {
+        // Compute a downscale clip when --screenshot-max-width or
+        // --screenshot-max-height is set and the source exceeds either bound.
+        // The smaller scale factor wins so both bounds are respected while
+        // preserving aspect ratio.
+        let clip: ScreenshotClip | undefined;
+        if (
+          screenshotMaxWidth !== undefined ||
+          screenshotMaxHeight !== undefined
+        ) {
+          const box = await getSourceBox(page, element, fullPage);
+          if (box) {
+            clip = computeDownscaleClip(
+              box,
+              screenshotMaxWidth,
+              screenshotMaxHeight,
+            );
+          }
+        }
+
+        if (clip) {
+          // page.screenshot with clip lets the CDP scale param downscale the
+          // capture for viewport, full-page and element shots alike. We rely on
+          // Puppeteer's default of captureBeyondViewport=true when a clip is
+          // present so element/full-page captures below the fold still work.
+          screenshot = await page.screenshot({
+            type: format,
+            quality,
+            optimizeForSpeed: true,
+            clip,
+          });
+        } else if (element) {
+          screenshot = await element.screenshot({
+            type: format,
+            quality,
+            optimizeForSpeed: true,
+          });
+        } else {
+          screenshot = await page.screenshot({
+            type: format,
+            fullPage,
+            quality,
+            optimizeForSpeed: true,
+          });
+        }
+      } finally {
+        void element?.dispose();
       }
 
       if (request.params.uid) {

tests/tools/screenshot.test.ts

@@ -8,7 +8,9 @@ import assert from 'node:assert';
 import {rm, stat, mkdir, chmod, writeFile} from 'node:fs/promises';
 import {tmpdir} from 'node:os';
 import {join} from 'node:path';
-import {describe, it} from 'node:test';
+import {describe, it, afterEach} from 'node:test';
+
+import sinon from 'sinon';
 
 import type {ParsedArguments} from '../../src/bin/chrome-devtools-mcp-cli-options.js';
 import {TextSnapshot} from '../../src/TextSnapshot.js';
@@ -33,6 +35,10 @@ function pngHeight(data: Buffer): number {
 }
 
 describe('screenshot', () => {
+  afterEach(() => {
+    sinon.restore();
+  });
+
   describe('browser_take_screenshot', () => {
     it('with default options', async () => {
       await withMcpContext(async (response, context) => {
@@ -197,6 +203,56 @@ describe('screenshot', () => {
       });
     });
 
+    it('disposes the element handle after an element screenshot', async () => {
+      await withMcpContext(async (response, context) => {
+        const fixture = screenshots.button;
+        const mcpPage = context.getSelectedMcpPage();
+        await mcpPage.pptrPage.setContent(fixture.html);
+        mcpPage.textSnapshot = await TextSnapshot.create(mcpPage);
+        const handle = await mcpPage.getElementByUid('1_1');
+        const disposeSpy = sinon.spy(handle, 'dispose');
+        sinon.stub(mcpPage, 'getElementByUid').resolves(handle);
+
+        await screenshotTool.handler(
+          {
+            params: {format: 'png', uid: '1_1'},
+            page: mcpPage,
+          },
+          response,
+          context,
+        );
+
+        sinon.assert.calledOnce(disposeSpy);
+      });
+    });
+
+    it('disposes the element handle when the capture fails', async () => {
+      await withMcpContext(async (response, context) => {
+        const fixture = screenshots.button;
+        const mcpPage = context.getSelectedMcpPage();
+        await mcpPage.pptrPage.setContent(fixture.html);
+        mcpPage.textSnapshot = await TextSnapshot.create(mcpPage);
+        const handle = await mcpPage.getElementByUid('1_1');
+        const disposeSpy = sinon.spy(handle, 'dispose');
+        sinon.stub(handle, 'screenshot').rejects(new Error('Capture failed'));
+        sinon.stub(mcpPage, 'getElementByUid').resolves(handle);
+
+        await assert.rejects(
+          screenshotTool.handler(
+            {
+              params: {format: 'png', uid: '1_1'},
+              page: mcpPage,
+            },
+            response,
+            context,
+          ),
+          /Capture failed/,
+        );
+
+        sinon.assert.calledOnce(disposeSpy);
+      });
+    });
+
     it('with filePath', async () => {
       await withMcpContext(async (response, context) => {
         const filePath = join(tmpdir(), 'test-screenshot.png');