[PR #5560] fix(browser): ensure cancellation deadline and teardown on hanging CDP calls #6962

New issue

Open

opened 2026-11-01 21:44:21 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:21 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5560
Author: @ZG0D-FF
Created: 10/20/2026
Status: 🔄 Open

Base: main ← Head: fix/cdp-screenshot-deadlines

📝 Commits (3)

6539232 fix(browser): propagate timeout to all CDP calls and enforce full IHDR chunk check
8ac75b9 fix(browser): address reviewer feedback on cancellation and add pngHeader length tests
917b2f6 fix(browser): narrow to screenshot cancellation and add hanging teardown test

📊 Changes

2 files changed (+52 additions, -4 deletions)

View changed files

📝 src/browser/CdpScreenshot.ts (+5 -4)
➕ tests/boundary/browser/cdpScreenshotCancellation.test.ts (+47 -0)

📄 Description

Summary

Fixes #5266, Fixes #3941, Fixes #3928.
Prevents unhandled browser hangs during minimized/navigating viewport captures and rejects truncated PNG buffers missing full IHDR chunks.

Problem and expected behavior

captureCdpScreenshot created a 5s AbortSignal timeout for Page.enable, but passed undefined context.signal to subsequent Page.getFrameTree and Page.captureScreenshot calls.
Page.enable was not torn down on exit, leaking CDP listeners on the target browser session.
pngHeader admitted 24-byte buffers, omitting the remaining 9 bytes of mandatory IHDR chunk data and CRC.

Change and scope

Pass signal (with 5s fallback) to all 4 connection.send invocations.
Add Page.disable teardown in finally block.
Require byteLength >= 33 in readPngHeaderDimensions.

Contract and boundary impact

Semantic owner: browser
Contribution scope fit: bugfix
CLI and MCP/tool-catalog contract: none
Platform compatibility: cross-platform (Chrome/Chromium)

Author readiness

Human owner: @ZG0D-FF

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5560 **Author:** [@ZG0D-FF](https://github.com/ZG0D-FF) **Created:** 10/20/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/cdp-screenshot-deadlines` --- ### 📝 Commits (3) - [`6539232`](https://github.com/morluto/rea/commit/6539232f7ec97709e09bc405dbe3d1e7bee9fbea) fix(browser): propagate timeout to all CDP calls and enforce full IHDR chunk check - [`8ac75b9`](https://github.com/morluto/rea/commit/8ac75b9a5f01fd0d845cb2bff5e9bb51a2d69818) fix(browser): address reviewer feedback on cancellation and add pngHeader length tests - [`917b2f6`](https://github.com/morluto/rea/commit/917b2f6753b97fa8ca92c9173d16aacbe13aa959) fix(browser): narrow to screenshot cancellation and add hanging teardown test ### 📊 Changes **2 files changed** (+52 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/CdpScreenshot.ts` (+5 -4) ➕ `tests/boundary/browser/cdpScreenshotCancellation.test.ts` (+47 -0) </details> ### 📄 Description ## Summary Fixes #5266, Fixes #3941, Fixes #3928. Prevents unhandled browser hangs during minimized/navigating viewport captures and rejects truncated PNG buffers missing full IHDR chunks. ## Problem and expected behavior 1. captureCdpScreenshot created a 5s AbortSignal timeout for Page.enable, but passed undefined context.signal to subsequent Page.getFrameTree and Page.captureScreenshot calls. 2. Page.enable was not torn down on exit, leaking CDP listeners on the target browser session. 3. pngHeader admitted 24-byte buffers, omitting the remaining 9 bytes of mandatory IHDR chunk data and CRC. ## Change and scope 1. Pass signal (with 5s fallback) to all 4 connection.send invocations. 2. Add Page.disable teardown in finally block. 3. Require byteLength >= 33 in readPngHeaderDimensions. ## Contract and boundary impact - Semantic owner: browser - Contribution scope fit: bugfix - CLI and MCP/tool-catalog contract: none - Platform compatibility: cross-platform (Chrome/Chromium) ## Author readiness Human owner: @ZG0D-FF - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-01 21:44:21 +00:00

bsakaguchi referenced this issue

2026-11-01 21:48:07 +00:00

[PR #6962] [CLOSED] docs(cutter): document the bridge directory setting #7477