morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2893] [MERGED] fix(browser): report cancellation after cleanup and source-map abort #5474

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:06:55 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2893
Author: @morluto
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/16/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: cursor/browser-cancellation-after-cleanup-1ffc

📝 Commits (10+)

d32287e fix(browser): report cancellation after cleanup and source-map abort
0e92dd8 fix(setup): keep Devin configuration JSON-formatted
de1d3f2 test(browser): keep cleanup cancellation coverage in its own suite
474b34c test(ghidra): drop the duplicated path join import
82bcf08 test(cli): type the retained skill directory pairs
a742936 Merge commit 'ebaaa79ca0' into HEAD
bfcc952 fix(browser): preserve source-map cancellation results
1ed755a Merge branch 'main' into cursor/browser-cancellation-after-cleanup-1ffc
49ada44 Merge branch 'main' into cursor/browser-cancellation-after-cleanup-1ffc
e1cafaf Merge remote-tracking branch 'origin/main' into sweep/2893

📊 Changes

5 files changed (+208 additions, -5 deletions)

View changed files

📝 tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts (+195 -0)
📝 tests/boundary/mcp/fridaLifecycleMcp.test.ts (+5 -4)
📝 tests/boundary/mcp/toolInputAdmission.test.ts (+1 -1)
📝 tests/fixtures/fakeCdpBrowser.ts (+5 -0)
📝 tests/fixtures/fakeCdpBrowserTypes.ts (+2 -0)

📄 Description

Summary

An interrupt that arrives while a CDP session is closing could still publish the captured page, session, WebMCP discovery, or screenshot. An interrupt during an approved source-map download was reported as a provider failure instead of cancellation. Both now return AnalysisCancelledError for the operation the caller requested.

This branch also includes the current main typecheck and lint repairs from the companion check-restoration change, so this pull request can pass check:fast on its own.

Problem and expected behavior

inspect-web-page, bundle analysis, session observation, WebMCP discovery, and screenshot capture await closeCdpTargetSession after the result is already built. Cleanup notices an aborted signal and closes the socket, then the caller publishes success.

fetchWebSourceMaps throws the abort reason. That reason is a DOMException named AbortError, so CdpBrowserProvider wrapped it as ProviderAdapterError and the CLI reported execution_failure while still exiting 130 or 143.

After this change, an abort during cleanup or during a source-map download returns AnalysisCancelledError for inspect_web_page, analyze_web_bundle, observe_web_session, discover_webmcp_tools, or capture_web_screenshot.

Change and scope

The browser provider owns this classification. After cleanup, an aborted caller signal replaces a result that was already built. providerError maps an AbortError to AnalysisCancelledError before it can become a provider-adapter failure. Deadline failures stay SourceMapDeadlineError and are not treated as caller cancellation.

Contribution scope fit: ordinary fix inside the existing passive CDP provider. Maintainer scope decision: not applicable. This does not add target execution, injection, or mutation.

Contract and boundary impact

Semantic owner and earliest changed stage: CdpBrowserProvider after session cleanup and at provider error classification.
Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: cancelled operations stay AnalysisCancelledError. No new tool or argument.
Provider, bridge, target-format, or platform compatibility: passive CDP only. The fake browser can leave a source-map response open so the regression does not depend on a timer.
Evidence, artifact, provenance, or reconstruction contract: a cancelled operation does not publish the page or bundle that cleanup interrupted.
Target selection, authority, upstream behavior, and cleanup details: cleanup still runs. The published result is cancellation when the caller signal aborted during that cleanup.
Generated metadata: none.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

The network contact is the existing approved source-map fetch. Cancellation stops that fetch.

Evidence and regression coverage

Tests added or updated: lifecycle coverage for an interrupt during page cleanup, session cleanup, and a hung source-map download.
Base reproduction: on the previous provider, aborting during Network.disable or during the source-map response still produced a successful result or ProviderAdapterError.
User-visible CLI/MCP output: callers that already map AnalysisCancelledError to code: "cancelled" now do so for these two windows. The CLI signal wrapper in #2879 is unchanged here.
Remaining proof gaps: real-browser interruption was not run. The fake CDP socket and HTTP server cover the same provider boundaries.

Validation performed

npm run test:focused -- tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts — 14 passed.
npm run check:fast — typecheck and lint passed.

Compatibility, safety, and release

No breaking contract change. Operations that previously returned success after an interrupt during cleanup now return cancellation. That matches the signal the caller already raised.

_{🔄 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/2893 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `cursor/browser-cancellation-after-cleanup-1ffc` --- ### 📝 Commits (10+) - [`d32287e`](https://github.com/morluto/rea/commit/d32287ebf63f9bd20aff090aa2bd59aa2772abd6) fix(browser): report cancellation after cleanup and source-map abort - [`0e92dd8`](https://github.com/morluto/rea/commit/0e92dd8e600f973a8324500cdba26f5402984b9e) fix(setup): keep Devin configuration JSON-formatted - [`de1d3f2`](https://github.com/morluto/rea/commit/de1d3f2a240308f2c84879f47a8fd8548ff6b975) test(browser): keep cleanup cancellation coverage in its own suite - [`474b34c`](https://github.com/morluto/rea/commit/474b34c570430e933166e3e4ea473fba946d041a) test(ghidra): drop the duplicated path join import - [`82bcf08`](https://github.com/morluto/rea/commit/82bcf0814d6863dabaca7db09d6a66fc59878031) test(cli): type the retained skill directory pairs - [`a742936`](https://github.com/morluto/rea/commit/a742936108c0eef5680ea93261093f87e05f9fd2) Merge commit 'ebaaa79ca02bde4a73013a2a9a781d0b07a57ee4' into HEAD - [`bfcc952`](https://github.com/morluto/rea/commit/bfcc9526a34cdf9e1632bf7fba755d2838fe0254) fix(browser): preserve source-map cancellation results - [`1ed755a`](https://github.com/morluto/rea/commit/1ed755ac60197f38837578893bbf7d7aeabdf718) Merge branch 'main' into cursor/browser-cancellation-after-cleanup-1ffc - [`49ada44`](https://github.com/morluto/rea/commit/49ada44d9be13d5970e03496e47a39ee1dacf1c4) Merge branch 'main' into cursor/browser-cancellation-after-cleanup-1ffc - [`e1cafaf`](https://github.com/morluto/rea/commit/e1cafaff72652f8839bf0ab407d36b21c87683b2) Merge remote-tracking branch 'origin/main' into sweep/2893 ### 📊 Changes **5 files changed** (+208 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts` (+195 -0) 📝 `tests/boundary/mcp/fridaLifecycleMcp.test.ts` (+5 -4) 📝 `tests/boundary/mcp/toolInputAdmission.test.ts` (+1 -1) 📝 `tests/fixtures/fakeCdpBrowser.ts` (+5 -0) 📝 `tests/fixtures/fakeCdpBrowserTypes.ts` (+2 -0) </details> ### 📄 Description  ## Summary An interrupt that arrives while a CDP session is closing could still publish the captured page, session, WebMCP discovery, or screenshot. An interrupt during an approved source-map download was reported as a provider failure instead of cancellation. Both now return `AnalysisCancelledError` for the operation the caller requested. This branch also includes the current `main` typecheck and lint repairs from the companion check-restoration change, so this pull request can pass `check:fast` on its own. ## Problem and expected behavior `inspect-web-page`, bundle analysis, session observation, WebMCP discovery, and screenshot capture await `closeCdpTargetSession` after the result is already built. Cleanup notices an aborted signal and closes the socket, then the caller publishes success. `fetchWebSourceMaps` throws the abort reason. That reason is a DOMException named `AbortError`, so `CdpBrowserProvider` wrapped it as `ProviderAdapterError` and the CLI reported `execution_failure` while still exiting 130 or 143. After this change, an abort during cleanup or during a source-map download returns `AnalysisCancelledError` for `inspect_web_page`, `analyze_web_bundle`, `observe_web_session`, `discover_webmcp_tools`, or `capture_web_screenshot`. ## Change and scope The browser provider owns this classification. After cleanup, an aborted caller signal replaces a result that was already built. `providerError` maps an `AbortError` to `AnalysisCancelledError` before it can become a provider-adapter failure. Deadline failures stay `SourceMapDeadlineError` and are not treated as caller cancellation. Contribution scope fit: ordinary fix inside the existing passive CDP provider. Maintainer scope decision: not applicable. This does not add target execution, injection, or mutation. ## Contract and boundary impact - Semantic owner and earliest changed stage: `CdpBrowserProvider` after session cleanup and at provider error classification. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: cancelled operations stay `AnalysisCancelledError`. No new tool or argument. - Provider, bridge, target-format, or platform compatibility: passive CDP only. The fake browser can leave a source-map response open so the regression does not depend on a timer. - Evidence, artifact, provenance, or reconstruction contract: a cancelled operation does not publish the page or bundle that cleanup interrupted. - Target selection, authority, upstream behavior, and cleanup details: cleanup still runs. The published result is cancellation when the caller signal aborted during that cleanup. - Generated metadata: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [x] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. The network contact is the existing approved source-map fetch. Cancellation stops that fetch. ## Evidence and regression coverage - Tests added or updated: lifecycle coverage for an interrupt during page cleanup, session cleanup, and a hung source-map download. - Base reproduction: on the previous provider, aborting during `Network.disable` or during the source-map response still produced a successful result or `ProviderAdapterError`. - User-visible CLI/MCP output: callers that already map `AnalysisCancelledError` to `code: "cancelled"` now do so for these two windows. The CLI signal wrapper in #2879 is unchanged here. - Remaining proof gaps: real-browser interruption was not run. The fake CDP socket and HTTP server cover the same provider boundaries. ## Validation performed - `npm run test:focused -- tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts` — 14 passed. - `npm run check:fast` — typecheck and lint passed. ## Compatibility, safety, and release No breaking contract change. Operations that previously returned success after an interrupt during cleanup now return cancellation. That matches the signal the caller already raised.  <div><a href="https://cursor.com/agents/bc-e320bc38-6035-49f1-a48a-b356ac8c1ffc?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-e320bc38-6035-49f1-a48a-b356ac8c1ffc&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:06:55 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#5474

No description provided.

Rows
Columns