morluto/rea

Fork 0

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

[PR #4088] [MERGED] fix(browser): report attached locale coverage limits #6049

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:11:27 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/browser-attached-locale-3395

📝 Commits (2)

41e9a0f fix(browser): report attached locale coverage limits
9b21575 test(browser): verify attached request-language preservation

📊 Changes

6 files changed (+118 additions, -12 deletions)

View changed files

📝 docs/browser-scenario-contract.md (+9 -0)
📝 scripts/fixtures/browser-verifier-site.mjs (+14 -1)
📝 scripts/lib/browser-scenario-environment-e2e.mjs (+44 -10)
📝 src/browser/BrowserScenarioPartialObservation.ts (+1 -0)
📝 src/browser/PlaywrightBrowserScenarioProvider.test.ts (+46 -0)
📝 src/domain/browserScenarioValues.ts (+4 -1)

📄 Description

Summary

Fixes #3395 by explicitly identifying the connected-browser locale limitation, an outcome requested in the report. Attached captures now say that locale controls Intl formatting while navigator.language(s) and Accept-Language remain caller-controlled, with launch mode as the recovery path. This change does not claim to force an attached browser's page language.

Problem and approach

An attached fr-FR scenario previously returned navigator.language=en-US and Intl fr-FR without identifying that mismatch. Launch mode supplies both. The shared capture projection now retains the specific requested-locale limitation in success, cancellation and cleanup-failure observations. The authored input schema and owning browser guide describe the same boundary; no repeated live probe or state cache is added to production.

A native Emulation.setUserAgentOverride({userAgent:"",acceptLanguage:"fr-FR"}) candidate was tested and rejected: Chromium 140 ignored its language override; Chrome for Testing 156 applied it but left the current document's cached navigator.languages at fr-FR after reset/disconnect when navigator.language was read first. Navigating before checking hid that leak. No candidate emulation code is in this diff. Separately, main already loses a caller's custom navigator.platform when an otherwise empty CDP session detaches; that independent native-CDP limitation is tracked in #4089 and is not a restoration guarantee of this PR.

Contract and boundary impact

Owner: shared browser capture projection, used for complete and retained partial observations. Provider emulation and cleanup behavior stay at their existing boundary.
Scope: ordinary correctness/coverage disclosure within the existing browser capability; new maintainer scope decision is not applicable. No new execution, injection, mutation, network call, installer or provider capability.
CLI/MCP: existing request/result shapes; one specific attached-capture limitation and locale schema description. Artifact completeness continues to cover selected observations, not equality of launch/connect environments. No launch-only limitation is added.
Evidence/provenance, target selection, credentials, resource ownership and generated metadata formats remain compatible. Ignored generated outputs are not committed.

Executed evidence

Linux public CLI on main 1b8a9c8: attached DOM has navigator en-US, Intl fr-FR; launch DOM has both fr-FR. Actual default caller language is restored; caller browser/page survive. Custom caller UA/language/client hints remain visible; the pre-existing platform-detach limitation above was reproduced on unmodified main and independently through CDP.
New regression against the unfixed shared producer: 1 failure, 18 controls/cases pass. Final tests cover launch/connect success, cancellation and cleanup-failure projections, with no limitation falsely assigned to launch.
Chrome for Testing 156.0.8078.4, actual CLI and stdio MCP: captured navigator values remain the independently observed caller values; the attached limitation is present; Intl, timezone, viewport and language state restore without navigating away first. Initialization failure, action failure, cancellation, same-target concurrency and external-page preservation are verified.
Review follow-up 9b2157544: the fixture now owns the actual received HTTP header and carries it into captured DOM; a separate uncached browser request establishes the baseline and verifies it again after cleanup. Actual CLI/MCP flows pass with both default en-US,en;q=0.9 and caller-configured de-DE,de;q=0.9. A real Fetch-domain fault changed only the navigation's outgoing header to fr-FR; the new assertion failed specifically on request_language while navigator values stayed unchanged. npm run check passed again. This follow-up changes test scripts only; the completed full-browser run and cost comparison below use the unchanged runtime commit 41e9a0f5.
Full real-browser verifier passed: network, source maps/script export/module identity, CLI/MCP transport, DOM, storage, service-worker cancellation/restoration, large PNG export, popup/shared-context isolation and launch/connect cleanup. Final process lineage is verified with no live descendants. This is Linux browser coverage; no Windows/macOS or Hopper/Ghidra runtime claim for this change.

Validation performed

npm run test:focused -- src/browser/PlaywrightBrowserScenarioProvider.test.ts src/browser/PlaywrightScenarioBrowser.test.ts src/browser/PlaywrightScenarioSession.test.ts src/browser/BrowserScenario.test.ts tests/boundary/mcp/browserScenarioMcp.test.ts tests/process-global/browserScenarioCleanup.test.ts — 71 passed.
npm run check — passed typecheck, lint, formatting, unused-code/metadata/module checks and architecture guards.
npm run docs:check — passed, including five documentation consistency tests.
REA_BROWSER_EXECUTABLE=... npm run verify:browser — complete real verification passed on the browser above.

Completed-task cost comparison

Parent 55cef2566 versus 41e9a0f5, Node 24.18.0/npm 11.16.0, same local task and Chrome binary. Two alternating samples per cache mode; baseline / fixed:

Node compile cache	Wall (s)	Reaped child CPU (s)	Sampled peak family RSS (MiB)
cold	3.45 / 3.32	4.03 / 3.97	1385.0 / 1382.7
warm	3.04 / 3.00	3.65 / 3.61	1385.1 / 1385.2

The measured task includes a fresh real browser, loopback fixture, public CLI DOM capture, independent page/request language and caller-state checks, and cleanup. Response size is 6,030 → 6,394 bytes; actual full CLI responses are 1,465–1,470 → 1,528–1,534 cl100k_base tokens (+about 63 tokens for the necessary limitation in the existing envelope). The actual full MCP tool-list reply is 811,850 → 811,880 tokens (+30), with the same 207 tools. No new production tool call, browser probe, retained state or cache is introduced.

Cold disables Node compile caching; warm primes per-version caches. Browsers are fresh in both modes; OS caches are uncontrolled. Shared host and n=2 do not establish a speedup. Wall excludes monitor polling delay; CPU includes reaped descendants; 200 ms family RSS samples may double-count shared pages or miss brief children. Token counts are retained-output/schema proxies; no model consumed them, and whole-agent-task tokens/billing are unmeasured.

Heavy checks ran serially on two CPU cores with bounded Node heaps and an own-family 7 GiB guard inside the authorized 8 GiB overall budget. Maximum measured validation family RSS was 4.59 GiB; no guard fired and session OOM counters remained zero.

Author readiness

Human owner: @N0zoM1z0

The human owner explicitly authorized delegated review, proxy completion of these declarations, ready-for-review publication and normal merge for this batch. The agent reviewed the final diff and evidence; this records that delegation and does not assert a separate manual review occurred.

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/4088 **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/browser-attached-locale-3395` --- ### 📝 Commits (2) - [`41e9a0f`](https://github.com/morluto/rea/commit/41e9a0f502d8abf4d61230b853beb1bb1b1d0716) fix(browser): report attached locale coverage limits - [`9b21575`](https://github.com/morluto/rea/commit/9b2157544e96483f4143d211bf7efde7482e35fd) test(browser): verify attached request-language preservation ### 📊 Changes **6 files changed** (+118 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `docs/browser-scenario-contract.md` (+9 -0) 📝 `scripts/fixtures/browser-verifier-site.mjs` (+14 -1) 📝 `scripts/lib/browser-scenario-environment-e2e.mjs` (+44 -10) 📝 `src/browser/BrowserScenarioPartialObservation.ts` (+1 -0) 📝 `src/browser/PlaywrightBrowserScenarioProvider.test.ts` (+46 -0) 📝 `src/domain/browserScenarioValues.ts` (+4 -1) </details> ### 📄 Description ## Summary Fixes #3395 by explicitly identifying the connected-browser locale limitation, an outcome requested in the report. Attached captures now say that locale controls Intl formatting while `navigator.language(s)` and `Accept-Language` remain caller-controlled, with launch mode as the recovery path. This change does **not** claim to force an attached browser's page language. ## Problem and approach An attached `fr-FR` scenario previously returned `navigator.language=en-US` and Intl `fr-FR` without identifying that mismatch. Launch mode supplies both. The shared capture projection now retains the specific requested-locale limitation in success, cancellation and cleanup-failure observations. The authored input schema and owning browser guide describe the same boundary; no repeated live probe or state cache is added to production. A native `Emulation.setUserAgentOverride({userAgent:"",acceptLanguage:"fr-FR"})` candidate was tested and rejected: Chromium 140 ignored its language override; Chrome for Testing 156 applied it but left the current document's cached `navigator.languages` at `fr-FR` after reset/disconnect when `navigator.language` was read first. Navigating before checking hid that leak. No candidate emulation code is in this diff. Separately, main already loses a caller's custom `navigator.platform` when an otherwise empty CDP session detaches; that independent native-CDP limitation is tracked in #4089 and is not a restoration guarantee of this PR. ## Contract and boundary impact - Owner: shared browser capture projection, used for complete and retained partial observations. Provider emulation and cleanup behavior stay at their existing boundary. - Scope: ordinary correctness/coverage disclosure within the existing browser capability; new maintainer scope decision is not applicable. No new execution, injection, mutation, network call, installer or provider capability. - CLI/MCP: existing request/result shapes; one specific attached-capture limitation and locale schema description. Artifact completeness continues to cover selected observations, not equality of launch/connect environments. No launch-only limitation is added. - Evidence/provenance, target selection, credentials, resource ownership and generated metadata formats remain compatible. Ignored generated outputs are not committed. ## Executed evidence - Linux public CLI on main `1b8a9c8`: attached DOM has navigator `en-US`, Intl `fr-FR`; launch DOM has both `fr-FR`. Actual default caller language is restored; caller browser/page survive. Custom caller UA/language/client hints remain visible; the pre-existing platform-detach limitation above was reproduced on unmodified main and independently through CDP. - New regression against the unfixed shared producer: **1 failure, 18 controls/cases pass**. Final tests cover launch/connect success, cancellation and cleanup-failure projections, with no limitation falsely assigned to launch. - **Chrome for Testing 156.0.8078.4, actual CLI and stdio MCP:** captured navigator values remain the independently observed caller values; the attached limitation is present; Intl, timezone, viewport and language state restore without navigating away first. Initialization failure, action failure, cancellation, same-target concurrency and external-page preservation are verified. - Review follow-up `9b2157544`: the fixture now owns the actual received HTTP header and carries it into captured DOM; a separate uncached browser request establishes the baseline and verifies it again after cleanup. Actual CLI/MCP flows pass with both default `en-US,en;q=0.9` and caller-configured `de-DE,de;q=0.9`. A real Fetch-domain fault changed only the navigation's outgoing header to `fr-FR`; the new assertion failed specifically on `request_language` while navigator values stayed unchanged. `npm run check` passed again. This follow-up changes test scripts only; the completed full-browser run and cost comparison below use the unchanged runtime commit `41e9a0f5`. - Full real-browser verifier passed: network, source maps/script export/module identity, CLI/MCP transport, DOM, storage, service-worker cancellation/restoration, large PNG export, popup/shared-context isolation and launch/connect cleanup. Final process lineage is verified with no live descendants. This is Linux browser coverage; no Windows/macOS or Hopper/Ghidra runtime claim for this change. ## Validation performed - `npm run test:focused -- src/browser/PlaywrightBrowserScenarioProvider.test.ts src/browser/PlaywrightScenarioBrowser.test.ts src/browser/PlaywrightScenarioSession.test.ts src/browser/BrowserScenario.test.ts tests/boundary/mcp/browserScenarioMcp.test.ts tests/process-global/browserScenarioCleanup.test.ts` — **71 passed**. - `npm run check` — passed typecheck, lint, formatting, unused-code/metadata/module checks and architecture guards. - `npm run docs:check` — passed, including five documentation consistency tests. - `REA_BROWSER_EXECUTABLE=... npm run verify:browser` — complete real verification passed on the browser above. ### Completed-task cost comparison Parent `55cef2566` versus `41e9a0f5`, Node 24.18.0/npm 11.16.0, same local task and Chrome binary. Two alternating samples per cache mode; **baseline / fixed**: | Node compile cache | Wall (s) | Reaped child CPU (s) | Sampled peak family RSS (MiB) | | --- | --- | --- | --- | | cold | 3.45 / 3.32 | 4.03 / 3.97 | 1385.0 / 1382.7 | | warm | 3.04 / 3.00 | 3.65 / 3.61 | 1385.1 / 1385.2 | The measured task includes a fresh real browser, loopback fixture, public CLI DOM capture, independent page/request language and caller-state checks, and cleanup. Response size is **6,030 → 6,394 bytes**; actual full CLI responses are **1,465–1,470 → 1,528–1,534 cl100k_base tokens** (+about 63 tokens for the necessary limitation in the existing envelope). The actual full MCP tool-list reply is **811,850 → 811,880 tokens** (+30), with the same 207 tools. No new production tool call, browser probe, retained state or cache is introduced. Cold disables Node compile caching; warm primes per-version caches. Browsers are fresh in both modes; OS caches are uncontrolled. Shared host and n=2 do not establish a speedup. Wall excludes monitor polling delay; CPU includes reaped descendants; 200 ms family RSS samples may double-count shared pages or miss brief children. Token counts are retained-output/schema proxies; no model consumed them, and whole-agent-task tokens/billing are unmeasured. Heavy checks ran serially on two CPU cores with bounded Node heaps and an own-family 7 GiB guard inside the authorized 8 GiB overall budget. Maximum measured validation family RSS was **4.59 GiB**; no guard fired and session OOM counters remained zero. ## Author readiness Human owner: @N0zoM1z0 The human owner explicitly authorized delegated review, proxy completion of these declarations, ready-for-review publication and normal merge for this batch. The agent reviewed the final diff and evidence; this records that delegation and does not assert a separate manual review occurred. - [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:11:27 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:48 +00:00

[PR #6143] [MERGED] fix(cancellation): preserve caller abort reasons through late finalization #7158

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#6049

No description provided.

Rows
Columns