morluto/rea

Fork 0

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

[PR #7187] [CLOSED] fix(browser): explain service-worker policy check failures in connect mode #7704

New issue

Closed

opened 2026-11-03 13:39:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:39:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7187
Author: @gcorces
Created: 10/26/2026
Status: ❌ Closed

Base: main ← Head: claude/dazzling-villani-458ddb

📝 Commits (1)

c649038 fix(browser): explain service-worker policy check failures in connect mode

📊 Changes

3 files changed (+77 additions, -3 deletions)

View changed files

📝 docs/browser-scenario-contract.md (+6 -1)
📝 scripts/lib/browser-scenario-service-workers-e2e.mjs (+51 -0)
📝 src/browser/PlaywrightScenarioServiceWorkers.ts (+20 -2)

📄 Description

Summary

capture_browser_scenario in connect mode with the default environment.service_workers: "block" returned a generic execution_failure ("Analysis could not complete. Run rea doctor…") with no cause, even with REA_LOG_LEVEL=debug, while the same scenario succeeded with allow. This was first observed against a real SaaS page in a Chrome started with --remote-debugging-port. The attached-page policy check now handles the two triggers reproduced locally. A sandboxed iframe is accepted as compliant. A document replaced during the check produces a typed target_changed refusal with its cause and remediation.

No issue is linked. The failure was found during local use and reproduced with local fixtures (see below).

Problem and expected behavior

PlaywrightScenarioServiceWorkers.assertBlocked evaluates a check in every frame of the attached page. Two inputs made frame.evaluate reject with a plain Playwright Error. assertBlocked rethrew it, so the error was wrapped as ProviderAdapterError and its reason was hidden:

Sandboxed iframe without allow-same-origin (no service worker anywhere). Reading navigator.serviceWorker in that opaque-origin document throws SecurityError: … Service worker is disabled because the context is sandboxed and lacks the 'allow-same-origin' flag. The documented contract is that block rejects pages or frames that are controlled by or have a registration. An opaque-origin document can be neither, so this was a false refusal.
A frame changes document during the check, for example a script redirect shortly after load. This gives frame.evaluate: Execution context was destroyed, most likely because of a navigation while the frame is still attached.

A page that does have a registration already produced the correct service_worker_blocked error with its remediation. That path is unchanged.

Change and scope

In the evaluated check, a SecurityError from the navigator.serviceWorker getter in an opaque-origin document (origin === "null") counts as policy-compliant. Any other getter failure is still rethrown.
A rejected evaluation on a still-attached frame now throws BrowserObservationError(capture_browser_scenario, "target_changed"). The error has a REA-authored detail, the original cause, and a remediation: retry after navigation settles, select the settled URL as start_url, or set environment.service_workers to allow. Page-controlled error text is not copied into the message.
docs/browser-scenario-contract.md documents both behaviors.

Intentionally not included:

Re-checking the replacement document instead of failing closed. I tried an immediate second pass. Chromium rejects the pending evaluation about 10 ms before Playwright processes the context events, so the retry reuses the dead realm and fails in about 5 ms. A reliable re-check would need to wait for a navigation signal under a deadline, which is a design decision left open here. frame.waitForFunction retries across navigations, but it uses page-level eval, which a strict CSP without 'unsafe-eval' blocks.
The connect-mode unreachable start_url case. It reports cleanup_incomplete, with frame.evaluate: Execution context was destroyed… under allow or protocol_error under block, and a generic primary execution_failure. It involves the cleanup lifecycle, and the real-browser lane already expects deferred restoration after a failed navigation, so I am drafting it as a separate issue.

Contribution scope: ordinary fix within existing boundaries. No maintainer scope decision is required.

Contract and boundary impact

Semantic owner and earliest changed stage: PlaywrightScenarioServiceWorkers.assertBlocked, the attached service-worker policy owner. Its consumers are the PlaywrightScenarioSession open path and the per-artifact policy brackets. Error projection is unchanged; the owner now produces a typed error instead of an untyped one.
Contribution scope fit and linked maintainer decision: not applicable (ordinary fix).
CLI and MCP/tool-catalog contract: no schema change. Opaque-origin frames no longer cause a refusal. Interrupted checks report code: target_unavailable, details.reason: target_changed instead of code: execution_failure.
Provider, bridge, target-format, or platform compatibility: relies on Chromium's SecurityError for opaque-origin documents. Verified on Chrome 155.0.8059.86.
Evidence, artifact, provenance, or reconstruction contract: none. Partial observations are retained as before.
Target selection, authority, upstream behavior, and cleanup details: unchanged. The e2e asserts disconnected-external cleanup and restored register after both new cases.
Generated metadata, package, or installation impact: none.

Expected effects:

Read existing files or captured data.
Launch or attach to a process. (existing connect attachment; unchanged)
Inject instrumentation or run scripts/code. (existing in-page policy check script; its logic changed)
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: scripts/lib/browser-scenario-service-workers-e2e.mjs (real-browser lane), with two fixture pages:
- /sandboxed-frame: a page with no registration and <iframe sandbox="allow-scripts">. The capture under block must complete with disconnected-external cleanup.
- /navigate-during-check: deterministic. The page overrides getRegistrations so the policy check's own lookup calls location.replace while the evaluation is pending. The public CLI must return code: target_unavailable, details.reason: target_changed, a remediation naming environment.service_workers, and disconnected-external cleanup in the partial observation.
Base reproduction (executed, Chrome 155, Windows 10, CLI capture-browser-scenario, connect mode, default block):
- sandboxed iframe, no registration: code: execution_failure, details only provider_id/operation.
- script redirect 30 ms after load: code: execution_failure (3/3). With allow: success.
- page with a registration: code: target_unavailable, reason: service_worker_blocked (already correct).
- Probing frame.evaluate directly gave the SecurityError and Execution context was destroyed messages quoted above.
- With the parent's worker active at /, an ordinary same-origin iframe was served by the worker, while the sandboxed iframe was served from the network.
Negative control (executed): with only the src/ change reverted and the e2e kept, verifyScenarioServiceWorkers fails at the sandboxed-frame case with ProviderAdapterError.

User-visible CLI/MCP output (after):

error: Browser scenario capture failed
code: target_unavailable
category: unavailable
message: "capture_browser_scenario could not complete the blocked service-worker policy check because a frame of the selected page changed document or rejected the check, so REA cannot confirm that no service worker controls the page."
remediation:
  action: "Retry after the page's navigation settles, or select the settled URL as start_url. Set environment.service_workers to allow if service workers may run during capture."
details:
  reason: target_changed

The sandboxed-iframe page now returns an evidence_id with completed steps.

Remaining proof gaps:
- The original SaaS page was not re-run, so it is unknown which of the two triggers it hit.
- The full npm run verify:browser lane and Linux CI were not run locally.

Boundary regressions: Public success and error delivery is covered through the CLI, including the partial observation on failure. For lifecycle, cleanup after both new outcomes is asserted (disconnected-external, native register restored). No ownership or finalization code changed.

For evidence-bearing changes:

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

Run on Windows 10 with Node 24.22.1. The final commit differs from the validated tree only in code-comment wording (lint-staged oxlint/oxfmt ran on it).

npm run check:fast — passed (typecheck, lint)
npm run test:local -- src/browser/PlaywrightScenarioSession.test.ts src/browser/PlaywrightBrowserScenarioProvider.test.ts src/domain/analysisErrorProjection.test.ts — 144 passed
npm run test:focused -- tests/boundary/browser/cdpFailedAdmission.test.ts tests/boundary/browser/cdpTargetDeadlines.test.ts tests/process-global/browserScenarioCleanup.test.ts — 33 passed, 1 failed: retries a late Playwright open after cancellation before accepting another capture. It fails the same way with this change reverted, and it uses launch mode, which never constructs the changed owner.
npm run docs:check — passed
verifyScenarioServiceWorkers(endpoint, targetId) from scripts/lib/browser-scenario-service-workers-e2e.mjs, run through a local harness against Chrome 155.0.8059.86 (isolated profile, --headless=new) — passed in 30.5 s
Same harness with the src/ change reverted — failed as expected (ProviderAdapterError)

Compatibility, safety, and release

Breaking changes or migration steps: none. A refusal that previously surfaced as execution_failure now surfaces as target_unavailable/target_changed. Pages whose only "violation" was an opaque-origin frame are now captured.
Real Hopper/Ghidra, browser, or OS coverage: real Chrome 155 on Windows for the service-worker lane block only. Not run on Linux.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the exemption is limited to opaque-origin documents, where Chromium disables the service-worker API. This was verified empirically for interception and is consistent with storage-key matching. The new error does not echo page-controlled exception text.

Author readiness

Human owner:

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.

🤖 Generated with Claude Code

_{🔄 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/7187 **Author:** [@gcorces](https://github.com/gcorces) **Created:** 10/26/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `claude/dazzling-villani-458ddb` --- ### 📝 Commits (1) - [`c649038`](https://github.com/morluto/rea/commit/c6490384ae87aac6838800f0004d437aab622f64) fix(browser): explain service-worker policy check failures in connect mode ### 📊 Changes **3 files changed** (+77 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `docs/browser-scenario-contract.md` (+6 -1) 📝 `scripts/lib/browser-scenario-service-workers-e2e.mjs` (+51 -0) 📝 `src/browser/PlaywrightScenarioServiceWorkers.ts` (+20 -2) </details> ### 📄 Description ## Summary `capture_browser_scenario` in `connect` mode with the default `environment.service_workers: "block"` returned a generic `execution_failure` ("Analysis could not complete. Run `rea doctor`…") with no cause, even with `REA_LOG_LEVEL=debug`, while the same scenario succeeded with `allow`. This was first observed against a real SaaS page in a Chrome started with `--remote-debugging-port`. The attached-page policy check now handles the two triggers reproduced locally. A sandboxed iframe is accepted as compliant. A document replaced during the check produces a typed `target_changed` refusal with its cause and remediation. No issue is linked. The failure was found during local use and reproduced with local fixtures (see below). ## Problem and expected behavior `PlaywrightScenarioServiceWorkers.assertBlocked` evaluates a check in every frame of the attached page. Two inputs made `frame.evaluate` reject with a plain Playwright `Error`. `assertBlocked` rethrew it, so the error was wrapped as `ProviderAdapterError` and its reason was hidden: 1. **Sandboxed iframe without `allow-same-origin`** (no service worker anywhere). Reading `navigator.serviceWorker` in that opaque-origin document throws `SecurityError: … Service worker is disabled because the context is sandboxed and lacks the 'allow-same-origin' flag.` The documented contract is that `block` rejects pages or frames that are *controlled by* or *have* a registration. An opaque-origin document can be neither, so this was a false refusal. 2. **A frame changes document during the check**, for example a script redirect shortly after `load`. This gives `frame.evaluate: Execution context was destroyed, most likely because of a navigation` while the frame is still attached. A page that does have a registration already produced the correct `service_worker_blocked` error with its remediation. That path is unchanged. ## Change and scope - In the evaluated check, a `SecurityError` from the `navigator.serviceWorker` getter in an opaque-origin document (`origin === "null"`) counts as policy-compliant. Any other getter failure is still rethrown. - A rejected evaluation on a still-attached frame now throws `BrowserObservationError(capture_browser_scenario, "target_changed")`. The error has a REA-authored detail, the original cause, and a remediation: retry after navigation settles, select the settled URL as `start_url`, or set `environment.service_workers` to `allow`. Page-controlled error text is not copied into the message. - `docs/browser-scenario-contract.md` documents both behaviors. Intentionally not included: - **Re-checking the replacement document instead of failing closed.** I tried an immediate second pass. Chromium rejects the pending evaluation about 10 ms before Playwright processes the context events, so the retry reuses the dead realm and fails in about 5 ms. A reliable re-check would need to wait for a navigation signal under a deadline, which is a design decision left open here. `frame.waitForFunction` retries across navigations, but it uses page-level `eval`, which a strict CSP without `'unsafe-eval'` blocks. - **The `connect`-mode unreachable `start_url` case.** It reports `cleanup_incomplete`, with `frame.evaluate: Execution context was destroyed…` under `allow` or `protocol_error` under `block`, and a generic primary `execution_failure`. It involves the cleanup lifecycle, and the real-browser lane already expects deferred restoration after a failed navigation, so I am drafting it as a separate issue. Contribution scope: ordinary fix within existing boundaries. No maintainer scope decision is required. ## Contract and boundary impact - Semantic owner and earliest changed stage: `PlaywrightScenarioServiceWorkers.assertBlocked`, the attached service-worker policy owner. Its consumers are the `PlaywrightScenarioSession` open path and the per-artifact policy brackets. Error projection is unchanged; the owner now produces a typed error instead of an untyped one. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix). - CLI and MCP/tool-catalog contract: no schema change. Opaque-origin frames no longer cause a refusal. Interrupted checks report `code: target_unavailable`, `details.reason: target_changed` instead of `code: execution_failure`. - Provider, bridge, target-format, or platform compatibility: relies on Chromium's `SecurityError` for opaque-origin documents. Verified on Chrome 155.0.8059.86. - Evidence, artifact, provenance, or reconstruction contract: none. Partial observations are retained as before. - Target selection, authority, upstream behavior, and cleanup details: unchanged. The e2e asserts `disconnected-external` cleanup and restored `register` after both new cases. - Generated metadata, package, or installation impact: none. Expected effects: - [ ] Read existing files or captured data. - [x] Launch or attach to a process. (existing `connect` attachment; unchanged) - [x] Inject instrumentation or run scripts/code. (existing in-page policy check script; its logic changed) - [ ] 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: `scripts/lib/browser-scenario-service-workers-e2e.mjs` (real-browser lane), with two fixture pages: - `/sandboxed-frame`: a page with no registration and `<iframe sandbox="allow-scripts">`. The capture under `block` must complete with `disconnected-external` cleanup. - `/navigate-during-check`: deterministic. The page overrides `getRegistrations` so the policy check's own lookup calls `location.replace` while the evaluation is pending. The public CLI must return `code: target_unavailable`, `details.reason: target_changed`, a remediation naming `environment.service_workers`, and `disconnected-external` cleanup in the partial observation. - Base reproduction (executed, Chrome 155, Windows 10, CLI `capture-browser-scenario`, `connect` mode, default `block`): - sandboxed iframe, no registration: `code: execution_failure`, details only `provider_id`/`operation`. - script redirect 30 ms after `load`: `code: execution_failure` (3/3). With `allow`: success. - page with a registration: `code: target_unavailable`, `reason: service_worker_blocked` (already correct). - Probing `frame.evaluate` directly gave the `SecurityError` and `Execution context was destroyed` messages quoted above. - With the parent's worker active at `/`, an ordinary same-origin iframe was served by the worker, while the sandboxed iframe was served from the network. - Negative control (executed): with only the `src/` change reverted and the e2e kept, `verifyScenarioServiceWorkers` fails at the sandboxed-frame case with `ProviderAdapterError`. - User-visible CLI/MCP output (after): ``` error: Browser scenario capture failed code: target_unavailable category: unavailable message: "capture_browser_scenario could not complete the blocked service-worker policy check because a frame of the selected page changed document or rejected the check, so REA cannot confirm that no service worker controls the page." remediation: action: "Retry after the page's navigation settles, or select the settled URL as start_url. Set environment.service_workers to allow if service workers may run during capture." details: reason: target_changed ``` The sandboxed-iframe page now returns an `evidence_id` with completed steps. - Remaining proof gaps: - The original SaaS page was not re-run, so it is unknown which of the two triggers it hit. - The full `npm run verify:browser` lane and Linux CI were not run locally. Boundary regressions: *Public success and error delivery* is covered through the CLI, including the partial observation on failure. For lifecycle, cleanup after both new outcomes is asserted (`disconnected-external`, native `register` restored). No ownership or finalization code changed. For evidence-bearing changes: - [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 Run on Windows 10 with Node 24.22.1. The final commit differs from the validated tree only in code-comment wording (lint-staged `oxlint`/`oxfmt` ran on it). - `npm run check:fast` — passed (typecheck, lint) - `npm run test:local -- src/browser/PlaywrightScenarioSession.test.ts src/browser/PlaywrightBrowserScenarioProvider.test.ts src/domain/analysisErrorProjection.test.ts` — 144 passed - `npm run test:focused -- tests/boundary/browser/cdpFailedAdmission.test.ts tests/boundary/browser/cdpTargetDeadlines.test.ts tests/process-global/browserScenarioCleanup.test.ts` — 33 passed, 1 failed: `retries a late Playwright open after cancellation before accepting another capture`. It fails the same way with this change reverted, and it uses `launch` mode, which never constructs the changed owner. - `npm run docs:check` — passed - `verifyScenarioServiceWorkers(endpoint, targetId)` from `scripts/lib/browser-scenario-service-workers-e2e.mjs`, run through a local harness against Chrome 155.0.8059.86 (isolated profile, `--headless=new`) — passed in 30.5 s - Same harness with the `src/` change reverted — failed as expected (`ProviderAdapterError`) ## Compatibility, safety, and release - Breaking changes or migration steps: none. A refusal that previously surfaced as `execution_failure` now surfaces as `target_unavailable`/`target_changed`. Pages whose only "violation" was an opaque-origin frame are now captured. - Real Hopper/Ghidra, browser, or OS coverage: real Chrome 155 on Windows for the service-worker lane block only. Not run on Linux. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the exemption is limited to opaque-origin documents, where Chromium disables the service-worker API. This was verified empirically for interception and is consistent with storage-key matching. The new error does not echo page-controlled exception text. ## Author readiness Human owner:  - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-03 13:39:46 +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#7704

No description provided.

Rows
Columns