[PR #2441] [MERGED] fix(browser): release scenario listeners and allow bounded Chrome close #5231

New issue

Closed

opened 2026-10-31 01:05:00 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:05:00 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2441
Author: @LcokHcak
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: codex/rea-browser-listener-cleanup

📝 Commits (5)

5d2928b fix(browser): release scenario event listeners on finish
b05ad25 fix(browser): allow Chrome cleanup to complete within bounded deadline
579b756 fix(browser): release listeners with their closed resource owners
4fa8a62 style(browser): format cleanup deadline regression
a101d8d test(browser): expect popup cleanup listener

📊 Changes

5 files changed (+263 additions, -36 deletions)

View changed files

📝 src/browser/PlaywrightScenarioBrowser.test.ts (+9 -1)
📝 src/browser/PlaywrightScenarioBrowser.ts (+8 -1)
➕ src/browser/PlaywrightScenarioEvents.sharedContext.test.ts (+130 -0)
📝 src/browser/PlaywrightScenarioEvents.ts (+98 -33)
📝 tests/process-global/browserScenarioCleanup.test.ts (+18 -1)

📄 Description

Summary

Release scenario event listeners when capture finishes and give an owned Chrome context a bounded 30-second close window before declaring cleanup failure.

Problem and expected behavior

After a connection-mode scenario finished, live page objects retained its page/context/worker/WebSocket listeners. In six controlled Chrome runs, context.close() took about 0.96–10.69 seconds; a one-second confirmation deadline could report cleanup_failed before profile removal began. A deadline-only branch still failed a real Chrome run, so the listener and close changes are presented together.

Change and scope

PlaywrightScenarioEvents.finish() detaches the listeners it owns. Browser cleanup awaits confirmed context close within 30 seconds and continues to report cleanup_failed if confirmation does not arrive. Existing target selection, network attribution and browser authority remain unchanged. No issue is linked because the behavior was reproduced with controlled local Chrome scenarios.

Contract and boundary impact

Semantic owner and earliest changed stage: scenario event collector owns its listeners; browser scenario provider owns context close and profile cleanup.
Contribution scope fit and linked maintainer decision: cleanup fix within the existing browser scenario capability; separate advance scope decision is not applicable.
CLI and MCP/tool-catalog contract: no new tool, schema or target authority.
Provider and platform compatibility: bounded wait applies to the existing owned Chrome context lifecycle.
Evidence and provenance: failed confirmation remains cleanup_failed; unattributable frameless shared-context requests remain excluded.
Generated metadata/package impact: none beyond existing browser runtime files.

Known workflow effects are those of an existing user-selected scenario: local Chrome launch/attach, selected page activity, loopback fixture traffic in verification and an owned temporary profile. The patch does not expand target selection or browser control.

Evidence and regression coverage

New tests assert listener release and a successful close that exceeds the former one-second deadline. Controlled failure and timing traces are retained in the local audit record. Real Chrome verification covers redirects, nested popups, shared-context isolation, CLI and stdio MCP. The 30-second limit is a local bound, not a guarantee for every platform; the internal cause of the slowest Chrome close remains unknown.

Observed timing and inferred cause are distinguished.
Selected target ownership and partial coverage remain explicit.
Timeout and unattributable request limitations remain visible.

Validation performed

On the standalone browser branch at 82cfcee: npm run build:cached and the real Chrome popup/shared-context/CLI/MCP lane passed. Three earlier consecutive real Chrome runs also passed without audit-temp profile residue.
On the combined branch at 37b7550: npm run check, npm run docs:check, npm run verify:test-discovery, 16 relevant Vitest tests and the real Chrome lane passed with all coverage flags true.
The branch was rebased without conflict onto 4773c15; npm run check passed on the combined branch at that base. The four intervening upstream commits did not change these browser lifecycle files.

Compatibility, safety, and release

Breaking changes or migration steps: none intended.
Real-provider coverage: local Windows Chrome; other browsers and platforms were not run here.
Security/privacy: preserves existing conservative request attribution and explicit cleanup failure; no vulnerability claim.

Before merge

Concrete reproduced problem and focused outcome.
Scope and authority reviewed against CONTRIBUTING.md.
No issue linked; controlled reproduction is described above.
Listener release, slow successful close and real browser paths covered.
Diff checked for secrets, unrelated cleanup and unsupported claims.

_{🔄 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/2441 **Author:** [@LcokHcak](https://github.com/LcokHcak) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/rea-browser-listener-cleanup` --- ### 📝 Commits (5) - [`5d2928b`](https://github.com/morluto/rea/commit/5d2928bab6eb255b165fd0cf51a4e71d5e2a34ee) fix(browser): release scenario event listeners on finish - [`b05ad25`](https://github.com/morluto/rea/commit/b05ad2580f9a1894b52f9ce9d55b54fb1820ce57) fix(browser): allow Chrome cleanup to complete within bounded deadline - [`579b756`](https://github.com/morluto/rea/commit/579b7568aaae6125725a32bec84f4eafbc6a0c14) fix(browser): release listeners with their closed resource owners - [`4fa8a62`](https://github.com/morluto/rea/commit/4fa8a623f6c6e2a43d632031b73f243af8085481) style(browser): format cleanup deadline regression - [`a101d8d`](https://github.com/morluto/rea/commit/a101d8db84ec29c2db0b081db43061b76249d63b) test(browser): expect popup cleanup listener ### 📊 Changes **5 files changed** (+263 additions, -36 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/PlaywrightScenarioBrowser.test.ts` (+9 -1) 📝 `src/browser/PlaywrightScenarioBrowser.ts` (+8 -1) ➕ `src/browser/PlaywrightScenarioEvents.sharedContext.test.ts` (+130 -0) 📝 `src/browser/PlaywrightScenarioEvents.ts` (+98 -33) 📝 `tests/process-global/browserScenarioCleanup.test.ts` (+18 -1) </details> ### 📄 Description ## Summary Release scenario event listeners when capture finishes and give an owned Chrome context a bounded 30-second close window before declaring cleanup failure. ## Problem and expected behavior After a connection-mode scenario finished, live page objects retained its page/context/worker/WebSocket listeners. In six controlled Chrome runs, `context.close()` took about 0.96–10.69 seconds; a one-second confirmation deadline could report `cleanup_failed` before profile removal began. A deadline-only branch still failed a real Chrome run, so the listener and close changes are presented together. ## Change and scope `PlaywrightScenarioEvents.finish()` detaches the listeners it owns. Browser cleanup awaits confirmed context close within 30 seconds and continues to report `cleanup_failed` if confirmation does not arrive. Existing target selection, network attribution and browser authority remain unchanged. No issue is linked because the behavior was reproduced with controlled local Chrome scenarios. ## Contract and boundary impact - Semantic owner and earliest changed stage: scenario event collector owns its listeners; browser scenario provider owns context close and profile cleanup. - Contribution scope fit and linked maintainer decision: cleanup fix within the existing browser scenario capability; separate advance scope decision is not applicable. - CLI and MCP/tool-catalog contract: no new tool, schema or target authority. - Provider and platform compatibility: bounded wait applies to the existing owned Chrome context lifecycle. - Evidence and provenance: failed confirmation remains `cleanup_failed`; unattributable frameless shared-context requests remain excluded. - Generated metadata/package impact: none beyond existing browser runtime files. Known workflow effects are those of an existing user-selected scenario: local Chrome launch/attach, selected page activity, loopback fixture traffic in verification and an owned temporary profile. The patch does not expand target selection or browser control. ## Evidence and regression coverage New tests assert listener release and a successful close that exceeds the former one-second deadline. Controlled failure and timing traces are retained in the local audit record. Real Chrome verification covers redirects, nested popups, shared-context isolation, CLI and stdio MCP. The 30-second limit is a local bound, not a guarantee for every platform; the internal cause of the slowest Chrome close remains unknown. - [x] Observed timing and inferred cause are distinguished. - [x] Selected target ownership and partial coverage remain explicit. - [x] Timeout and unattributable request limitations remain visible. ## Validation performed - On the standalone browser branch at `82cfcee`: `npm run build:cached` and the real Chrome popup/shared-context/CLI/MCP lane passed. Three earlier consecutive real Chrome runs also passed without audit-temp profile residue. - On the combined branch at `37b7550`: `npm run check`, `npm run docs:check`, `npm run verify:test-discovery`, 16 relevant Vitest tests and the real Chrome lane passed with all coverage flags true. - The branch was rebased without conflict onto `4773c15`; `npm run check` passed on the combined branch at that base. The four intervening upstream commits did not change these browser lifecycle files. ## Compatibility, safety, and release - Breaking changes or migration steps: none intended. - Real-provider coverage: local Windows Chrome; other browsers and platforms were not run here. - Security/privacy: preserves existing conservative request attribution and explicit cleanup failure; no vulnerability claim. ## Before merge - [x] Concrete reproduced problem and focused outcome. - [x] Scope and authority reviewed against `CONTRIBUTING.md`. - [x] No issue linked; controlled reproduction is described above. - [x] Listener release, slow successful close and real browser paths covered. - [x] Diff checked for secrets, unrelated cleanup and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:05:00 +00:00