[PR #517] [MERGED] fix(browser): release cancelled CDP cleanup without waiting for responses #2043

New issue

Closed

opened 2026-10-16 21:13:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:13:39 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/517
Author: @rudycelekli
Created: 10/5/2026
Status: ✅ Merged
Merged: 10/5/2026
Merged by: @morluto

Base: main ← Head: fix/browser-cancel-cleanup-20261004

📝 Commits (1)

268f4fd fix(browser): release cancelled CDP cleanup without waiting for responses

📊 Changes

3 files changed (+130 additions, -7 deletions)

View changed files

📝 src/browser/CdpBrowserProvider.ts (+20 -4)
📝 src/browser/CdpTargetSession.ts (+38 -3)
➕ tests/boundary/browser/cdpCleanupCancellation.test.ts (+72 -0)

📄 Description

Change

Summary

Finish cancelled browser operations without waiting indefinitely for cleanup command acknowledgements. Keep domain-disable and session-detach sends, then close REA's owned connection and await settlement of its pending requests. Noncancelled cleanup still awaits each response. Sequential cleanup sends observe cancellation too, so cancellation arriving after capture completes also releases teardown.

A cancelled observation currently waits forever if Page.disable or Target.detachFromTarget receives no response, preventing the public cancellation result from returning. Forward the operation signal at all four existing cleanup callers and use the cancellation-only close path.

Verification

Actual owned HTTP/WebSocket transport fixture through production CdpBrowserProvider.observeSession: all three pre-fix cases hit the bounded test deadline (unanswered disable/detach with an already-cancelled observation, and cancellation during normal cleanup).
After: 15 tests pass across cleanup cancellation, existing lifecycle failures, and CDP correlation/cancellation controls.
Both unanswered disable and unanswered detach return the original cancellation; late cancellation retains the already-completed observation while releasing its transport; detach is sent and neither Browser.close nor Target.closeTarget is sent.
Pinned TypeScript and official repository lint pass.
Full repository/package gates recorded separately before publication.

This is real local transport boundary verification with simulated CDP responses. It does not claim a real Chromium application observation.

Reproduction and verification

3 failures (bounded test deadlines), corrected real public observation inputs

npm run check:pr: passed (Test Files 353 passed (353); Tests 1801 passed | 2 skipped (1803)).
npm run check:ci: passed.
npm run verify:package: passed installed-tarball CLI/MCP verification, including optional-dependency degradation.
npm pack --dry-run: passed.
Matching npm run verify:browser: passed the native verification lane.
Exact lockfile installation on Node 24.18.0 and npm 11.16.0.

The focused cancellation regressions use the actual production HTTP/WebSocket CDP transport with controlled replies; they are distinct from the passing real-Chrome verification lane above. No real Hopper/Ghidra or Linux sandbox end-to-end coverage is claimed.

Prepared with AI assistance; the reproductions and verification commands above were executed locally.

_{🔄 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/517 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/browser-cancel-cleanup-20261004` --- ### 📝 Commits (1) - [`268f4fd`](https://github.com/morluto/rea/commit/268f4fdb04bac0ceb4fd66bd9b913b8ee32c4cef) fix(browser): release cancelled CDP cleanup without waiting for responses ### 📊 Changes **3 files changed** (+130 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/CdpBrowserProvider.ts` (+20 -4) 📝 `src/browser/CdpTargetSession.ts` (+38 -3) ➕ `tests/boundary/browser/cdpCleanupCancellation.test.ts` (+72 -0) </details> ### 📄 Description ## Change ## Summary Finish cancelled browser operations without waiting indefinitely for cleanup command acknowledgements. Keep domain-disable and session-detach sends, then close REA's owned connection and await settlement of its pending requests. Noncancelled cleanup still awaits each response. Sequential cleanup sends observe cancellation too, so cancellation arriving after capture completes also releases teardown. A cancelled observation currently waits forever if `Page.disable` or `Target.detachFromTarget` receives no response, preventing the public cancellation result from returning. Forward the operation signal at all four existing cleanup callers and use the cancellation-only close path. ## Verification - Actual owned HTTP/WebSocket transport fixture through production `CdpBrowserProvider.observeSession`: all three pre-fix cases hit the bounded test deadline (unanswered disable/detach with an already-cancelled observation, and cancellation during normal cleanup). - After: 15 tests pass across cleanup cancellation, existing lifecycle failures, and CDP correlation/cancellation controls. - Both unanswered disable and unanswered detach return the original cancellation; late cancellation retains the already-completed observation while releasing its transport; detach is sent and neither Browser.close nor Target.closeTarget is sent. - Pinned TypeScript and official repository lint pass. - Full repository/package gates recorded separately before publication. This is real local transport boundary verification with simulated CDP responses. It does not claim a real Chromium application observation. ## Reproduction and verification 3 failures (bounded test deadlines), corrected real public observation inputs - `npm run check:pr`: passed (Test Files 353 passed (353); Tests 1801 passed | 2 skipped (1803)). - `npm run check:ci`: passed. - `npm run verify:package`: passed installed-tarball CLI/MCP verification, including optional-dependency degradation. - `npm pack --dry-run`: passed. Matching `npm run verify:browser`: passed the native verification lane. - Exact lockfile installation on Node 24.18.0 and npm 11.16.0. The focused cancellation regressions use the actual production HTTP/WebSocket CDP transport with controlled replies; they are distinct from the passing real-Chrome verification lane above. No real Hopper/Ghidra or Linux sandbox end-to-end coverage is claimed. Prepared with AI assistance; the reproductions and verification commands above were executed locally. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:13:39 +00:00