[PR #4010] [MERGED] fix(browser): confirm service-worker cancellation cleanup #5989

Closed
opened 2026-10-31 01:10:59 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4010
Author: @N0zoM1z0
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/windows-edge-service-worker-navigation-3816


📝 Commits (10+)

  • 12c6d95 fix(browser): confirm service-worker cancellation cleanup
  • df7a223 Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816
  • bfc0545 Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816
  • 912cdd4 fix(browser): restore policy after setup failure
  • b775324 fix(browser): transfer cancelled navigation cleanup
  • b900087 Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816
  • 49e03fd fix(browser): retain cancellation ownership across retries
  • dcd4b85 Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816
  • a1f1b96 Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816
  • 918bbd1 Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816

📊 Changes

8 files changed (+700 additions, -67 deletions)

View changed files

📝 scripts/lib/browser-scenario-storage-e2e.mjs (+11 -6)
📝 src/browser/CdpConnection.ts (+11 -1)
📝 src/browser/PlaywrightScenarioServiceWorkers.ts (+116 -27)
📝 src/browser/PlaywrightScenarioSession.test.ts (+405 -13)
📝 src/browser/PlaywrightScenarioSession.ts (+20 -4)
📝 src/browser/PlaywrightScenarioStorage.test.ts (+56 -9)
📝 src/browser/PlaywrightScenarioStorage.ts (+17 -7)
📝 tests/boundary/browser/cdpConnection.test.ts (+64 -0)

📄 Description

Summary

Attached Windows browser cancellation could exceed the service-worker owner's cleanup window, race two Page.stopLoading owners, and then turn Chromium's terminal detached-session response into a connection-wide protocol failure. This change gives each side effect and target lifetime one owner, restores the selected realm through its owned CDP session, and makes the real verifier require typed cancellation and close its provider.

Fixes #3816.

Problem and expected behavior

Cancelling a storage seed while Edge was navigating intermittently made the next service-worker scenario fail or stall. Cleanup must confirm restoration within a bounded interval, preserve a typed cancellation result, retain retry ownership on a real cleanup failure, and leave the attached target reusable.

Change and scope

  • Assign navigation stopping to storage when that owner exists; policy cleanup keeps the no-storage fallback.
  • Transfer stopping to the policy owner after a failed storage cleanup, once the storage attempt is no longer in flight.
  • Retain transferred stop ownership across cleanup retries until one owner confirms it, without repeating an already confirmed stop.
  • Make storage return explicit stop confirmation; transfer to policy when root discovery has not completed or a late stop arrives after storage release.
  • Track target detach explicitly so vanished iframe initialization cannot hold cleanup open.
  • After setup failure, clean up effects that were acquired before the failure instead of replaying the rejected setup promise on every retry.
  • Remove future-document scripts, restore the selected main realm through its owned CDP session, and restore only committed live child realms.
  • Restore the selected main realm through Playwright when its owned CDP session detaches unexpectedly; keep cleanup retryable when restoration cannot be confirmed.
  • Preserve Chromium's exact terminal -32001 Session with given id not found. as a command rejection for the matching connection-global command id; keep other missing or foreign session responses fail-closed.
  • Give the complete policy restoration chain a bounded five-second confirmation interval.
  • Make the real storage cancellation verifier close its provider and reject unrelated failures.

Contribution scope: ordinary fix within the existing read-only attached-browser capability; no new authority or provider. No maintainer scope decision is required.

Contract and boundary impact

  • Semantic owner and earliest changed stage: attached browser-session cleanup and the CDP response parser.
  • Contribution scope fit and linked maintainer decision: existing browser observation and cleanup semantics; not applicable.
  • CLI and MCP/tool-catalog contract: unchanged.
  • Provider, bridge, target-format, or platform compatibility: handles an observed Chromium flat-session terminal response; other responses retain strict validation.
  • Evidence, artifact, provenance, or reconstruction contract: unchanged.
  • Target selection, authority, upstream behavior, and cleanup details: selected target is unchanged; existing temporary service-worker policy is restored before disconnect, with retained retry ownership on unconfirmed live cleanup.
  • Generated metadata, package, or installation impact: 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.

Evidence and regression coverage

  • Tests added or updated: producer stop confirmation with pending root discovery and detach retry, late cancellation after storage release, single stop-loading ownership and failure transfer, retained ownership across session/setup cleanup retries, no duplicate confirmed stop, detached-main Playwright fallback with failed-restoration retry, no-storage fallback, cleanup past 1.1 seconds, setup failure after script acquisition, detach during initialization, empty uncommitted child frame, exact Chromium terminal rejection, and four strict near-miss response cases.

  • Base reproduction: ordinary-user Windows Edge 154.0.4258.48 reproduced the storage-cancellation to service-worker failure chain; retained diagnostic lanes isolate each boundary.

  • User-visible CLI/MCP output: no contract or result-shape change.

  • Remaining proof gaps: real Edge coverage is Windows only; Chrome and other operating systems were not independently rerun for this issue. Agent-token cost was not measured.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:focused -- src/browser/PlaywrightScenarioStorage.test.ts src/browser/PlaywrightScenarioSession.test.ts tests/boundary/browser/cdpConnection.test.ts tests/boundary/process/browserEnvironmentVerifier.test.ts — 57/57 passed on final latest-main head c45901a28ca25f296415178a481161da46ba9cf9, based on main c7697595d8e1b54ab04ba661f98ad595d0993c34.
  • npm run check:fast — typecheck and lint passed.
  • Ordinary-user Windows Edge storage→service-worker production workflow on exact head dcd4b85a5aeba08705c76c6544907376bd00e6b0 — all storage and service-worker assertions passed, including stalled image/frame cancellation, navigation during restoration, OOPIF policy setup, target reuse, and strict typed cancellation; cleanup left zero Job processes. The preceding r21 reached the service-worker phase without an assertion failure but exhausted its 420-second wall budget and terminated with zero Job processes.
  • The subsequent main refresh to c9f9b83c6 changed only exported-binding test fixtures and Kuna implementation, tests, verifier, skill, and documentation. Browser sources, verifier inputs, package inputs, and their blobs remained identical on intermediate head 918bbd1afb29c5cf284aaac9ba3055e9d7381233; 45/45 focused tests and check:fast passed there.
  • After the explicit storage-confirmation review fix, ordinary-user Windows Edge r23 passed on exact head ae9155c6ae4c42f9ee26d7c04a57c7ce64d72859: all storage and service-worker assertions passed and cleanup left Job process count zero.
  • Main then changed the real browser fixture and scenario limitations. After merging it, exact latest-main head c45901a28ca25f296415178a481161da46ba9cf9 passed ordinary-user Windows Edge r24 with every production assertion true and Job0.

The exact-head r22 workflow used 347.91 seconds of Job wall time with 295.80 sampled family CPU seconds, 1,020,522,496-byte peak Job commit, 1,237,225,472-byte peak sampled family working set, and Job process count zero. It used two logical CPUs, BelowNormal priority, and a 2 GiB Job limit. Focused traces measured the removed uncommitted-frame wait at about 10.4 seconds and direct main-realm CDP restoration at roughly 2–6 ms; passing runs have varied from about 203 to 370 seconds, so whole-run host variance does not support a speedup claim.

The final r23 workflow used 458.18 seconds of Job wall time with 482.72 sampled family CPU seconds, 1,240,961,024-byte peak Job commit, 1,379,008,512-byte peak sampled family working set, and Job process count zero under the same two logical CPUs, BelowNormal priority, and 2 GiB Job limit. The expanded timing range is reported as variance; no whole-task speedup is claimed.

The exact latest-main r24 workflow used 154.51 seconds of Job wall time with 130.69 sampled family CPU seconds, 1,137,291,264-byte peak Job commit, 1,331,421,184-byte peak sampled family working set, and Job process count zero under the same limits. This single faster run remains within the observed host variance and is not a speedup claim.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: Windows Edge only for this change; no Hopper/Ghidra path is affected.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new target authority; exact target selection and cleanup ownership are preserved, and uncertain cleanup remains retryable.

Author readiness

Human owner: @N0zoM1z0

  • 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.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff 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/4010 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/windows-edge-service-worker-navigation-3816` --- ### 📝 Commits (10+) - [`12c6d95`](https://github.com/morluto/rea/commit/12c6d9549055fb3de13afbb162ee50b224da64cb) fix(browser): confirm service-worker cancellation cleanup - [`df7a223`](https://github.com/morluto/rea/commit/df7a223f2879d2c7c15fb47f579bf7a4cf0d63a6) Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816 - [`bfc0545`](https://github.com/morluto/rea/commit/bfc0545b1b641edf7194909443e7ed300cf4c7df) Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816 - [`912cdd4`](https://github.com/morluto/rea/commit/912cdd49d537930adfed0f5a49816b8b0db6b4f5) fix(browser): restore policy after setup failure - [`b775324`](https://github.com/morluto/rea/commit/b775324ead8f8fb216a9c2ffa0f852803183fc05) fix(browser): transfer cancelled navigation cleanup - [`b900087`](https://github.com/morluto/rea/commit/b900087bf1818802accf725168231a0b1e429d42) Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816 - [`49e03fd`](https://github.com/morluto/rea/commit/49e03fd49ae0643c0c3a9f7c8f127849a482bbb9) fix(browser): retain cancellation ownership across retries - [`dcd4b85`](https://github.com/morluto/rea/commit/dcd4b85a5aeba08705c76c6544907376bd00e6b0) Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816 - [`a1f1b96`](https://github.com/morluto/rea/commit/a1f1b96a2999ded42c47d9a0dfbe742f4b1d1a79) Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816 - [`918bbd1`](https://github.com/morluto/rea/commit/918bbd1afb29c5cf284aaac9ba3055e9d7381233) Merge remote-tracking branch 'origin/main' into fix/windows-edge-service-worker-navigation-3816 ### 📊 Changes **8 files changed** (+700 additions, -67 deletions) <details> <summary>View changed files</summary> 📝 `scripts/lib/browser-scenario-storage-e2e.mjs` (+11 -6) 📝 `src/browser/CdpConnection.ts` (+11 -1) 📝 `src/browser/PlaywrightScenarioServiceWorkers.ts` (+116 -27) 📝 `src/browser/PlaywrightScenarioSession.test.ts` (+405 -13) 📝 `src/browser/PlaywrightScenarioSession.ts` (+20 -4) 📝 `src/browser/PlaywrightScenarioStorage.test.ts` (+56 -9) 📝 `src/browser/PlaywrightScenarioStorage.ts` (+17 -7) 📝 `tests/boundary/browser/cdpConnection.test.ts` (+64 -0) </details> ### 📄 Description ## Summary Attached Windows browser cancellation could exceed the service-worker owner's cleanup window, race two `Page.stopLoading` owners, and then turn Chromium's terminal detached-session response into a connection-wide protocol failure. This change gives each side effect and target lifetime one owner, restores the selected realm through its owned CDP session, and makes the real verifier require typed cancellation and close its provider. Fixes #3816. ## Problem and expected behavior Cancelling a storage seed while Edge was navigating intermittently made the next service-worker scenario fail or stall. Cleanup must confirm restoration within a bounded interval, preserve a typed cancellation result, retain retry ownership on a real cleanup failure, and leave the attached target reusable. ## Change and scope - Assign navigation stopping to storage when that owner exists; policy cleanup keeps the no-storage fallback. - Transfer stopping to the policy owner after a failed storage cleanup, once the storage attempt is no longer in flight. - Retain transferred stop ownership across cleanup retries until one owner confirms it, without repeating an already confirmed stop. - Make storage return explicit stop confirmation; transfer to policy when root discovery has not completed or a late stop arrives after storage release. - Track target detach explicitly so vanished iframe initialization cannot hold cleanup open. - After setup failure, clean up effects that were acquired before the failure instead of replaying the rejected setup promise on every retry. - Remove future-document scripts, restore the selected main realm through its owned CDP session, and restore only committed live child realms. - Restore the selected main realm through Playwright when its owned CDP session detaches unexpectedly; keep cleanup retryable when restoration cannot be confirmed. - Preserve Chromium's exact terminal `-32001 Session with given id not found.` as a command rejection for the matching connection-global command id; keep other missing or foreign session responses fail-closed. - Give the complete policy restoration chain a bounded five-second confirmation interval. - Make the real storage cancellation verifier close its provider and reject unrelated failures. Contribution scope: ordinary fix within the existing read-only attached-browser capability; no new authority or provider. No maintainer scope decision is required. ## Contract and boundary impact - Semantic owner and earliest changed stage: attached browser-session cleanup and the CDP response parser. - Contribution scope fit and linked maintainer decision: existing browser observation and cleanup semantics; not applicable. - CLI and MCP/tool-catalog contract: unchanged. - Provider, bridge, target-format, or platform compatibility: handles an observed Chromium flat-session terminal response; other responses retain strict validation. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Target selection, authority, upstream behavior, and cleanup details: selected target is unchanged; existing temporary service-worker policy is restored before disconnect, with retained retry ownership on unconfirmed live cleanup. - Generated metadata, package, or installation impact: none. Expected effects: - [ ] Read existing files or captured data. - [x] Launch or attach to a process. - [x] Inject instrumentation or run scripts/code. - [x] 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. ## Evidence and regression coverage - Tests added or updated: producer stop confirmation with pending root discovery and detach retry, late cancellation after storage release, single stop-loading ownership and failure transfer, retained ownership across session/setup cleanup retries, no duplicate confirmed stop, detached-main Playwright fallback with failed-restoration retry, no-storage fallback, cleanup past 1.1 seconds, setup failure after script acquisition, detach during initialization, empty uncommitted child frame, exact Chromium terminal rejection, and four strict near-miss response cases. - Base reproduction: ordinary-user Windows Edge 154.0.4258.48 reproduced the storage-cancellation to service-worker failure chain; retained diagnostic lanes isolate each boundary. - User-visible CLI/MCP output: no contract or result-shape change. - Remaining proof gaps: real Edge coverage is Windows only; Chrome and other operating systems were not independently rerun for this issue. Agent-token cost was not measured. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- src/browser/PlaywrightScenarioStorage.test.ts src/browser/PlaywrightScenarioSession.test.ts tests/boundary/browser/cdpConnection.test.ts tests/boundary/process/browserEnvironmentVerifier.test.ts` — 57/57 passed on final latest-main head `c45901a28ca25f296415178a481161da46ba9cf9`, based on main `c7697595d8e1b54ab04ba661f98ad595d0993c34`. - `npm run check:fast` — typecheck and lint passed. - Ordinary-user Windows Edge storage→service-worker production workflow on exact head `dcd4b85a5aeba08705c76c6544907376bd00e6b0` — all storage and service-worker assertions passed, including stalled image/frame cancellation, navigation during restoration, OOPIF policy setup, target reuse, and strict typed cancellation; cleanup left zero Job processes. The preceding r21 reached the service-worker phase without an assertion failure but exhausted its 420-second wall budget and terminated with zero Job processes. - The subsequent main refresh to `c9f9b83c6` changed only exported-binding test fixtures and Kuna implementation, tests, verifier, skill, and documentation. Browser sources, verifier inputs, package inputs, and their blobs remained identical on intermediate head `918bbd1afb29c5cf284aaac9ba3055e9d7381233`; 45/45 focused tests and `check:fast` passed there. - After the explicit storage-confirmation review fix, ordinary-user Windows Edge r23 passed on exact head `ae9155c6ae4c42f9ee26d7c04a57c7ce64d72859`: all storage and service-worker assertions passed and cleanup left Job process count zero. - Main then changed the real browser fixture and scenario limitations. After merging it, exact latest-main head `c45901a28ca25f296415178a481161da46ba9cf9` passed ordinary-user Windows Edge r24 with every production assertion true and Job0. The exact-head r22 workflow used 347.91 seconds of Job wall time with 295.80 sampled family CPU seconds, 1,020,522,496-byte peak Job commit, 1,237,225,472-byte peak sampled family working set, and Job process count zero. It used two logical CPUs, BelowNormal priority, and a 2 GiB Job limit. Focused traces measured the removed uncommitted-frame wait at about 10.4 seconds and direct main-realm CDP restoration at roughly 2–6 ms; passing runs have varied from about 203 to 370 seconds, so whole-run host variance does not support a speedup claim. The final r23 workflow used 458.18 seconds of Job wall time with 482.72 sampled family CPU seconds, 1,240,961,024-byte peak Job commit, 1,379,008,512-byte peak sampled family working set, and Job process count zero under the same two logical CPUs, BelowNormal priority, and 2 GiB Job limit. The expanded timing range is reported as variance; no whole-task speedup is claimed. The exact latest-main r24 workflow used 154.51 seconds of Job wall time with 130.69 sampled family CPU seconds, 1,137,291,264-byte peak Job commit, 1,331,421,184-byte peak sampled family working set, and Job process count zero under the same limits. This single faster run remains within the observed host variance and is not a speedup claim. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Windows Edge only for this change; no Hopper/Ghidra path is affected. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new target authority; exact target selection and cleanup ownership are preserved, and uncertain cleanup remains retryable. ## Author readiness Human owner: @N0zoM1z0 - [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. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff 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:10:59 +00:00
Sign in to join this conversation.
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#5989
No description provided.