morluto/rea

Fork 0

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

[PR #2885] [CLOSED] test(browser): preserve page URL during subresource redirects #5468

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2885
Author: @zhangguiping-xydt
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/browser-redirect-final-url

📝 Commits (4)

fd815a9 fix(browser): clear the final URL after an excluded redirect
0c43cb9 Merge remote-tracking branch 'origin/main' into HEAD
127fa75 chore(browser): integrate upstream CI repairs for redirect observation
9b4e5f4 test(browser): retain redirect scope coverage with upstream fixes

📊 Changes

1 file changed (+1 additions, -0 deletions)

View changed files

📝 tests/boundary/browser/cdpObservationResourceTypes.test.ts (+1 -0)

📄 Description

What Problem This Solves

Refs #2593. Browser observation must distinguish a main-frame redirect leaving the approved origins from a cross-origin subresource redirect. The former makes the final page URL unknown; Fetch/Script redirects must preserve the approved page URL.

Why This Change Was Made

Main now includes the original runtime correction in b98be5408: excludeTargetScope() records exclusion, clears finalUrl, and prevents later events or final-frame capture from restoring a stale approved URL. This branch incorporates that implementation and its document-redirect assertions instead of keeping a duplicate assignment.

The final diff against main adds one assertion to the existing Fetch/Script negative controls: target.final_url must equal target.initial_url. Main-frame exclusion and omitted resource-type cases remain covered by upstream tests. Existing maintainer and PR history is preserved.

User Impact

Protects the existing observe-web-session and observe_web_session behavior: report unknown final locations after scope escape while retaining confirmed page locations after unrelated subresource redirects. No new runtime, schema, dependency, authority or release changes.

Evidence

Current commit: 9b4e5f43a3b0d3400b1baa6fa660f53ff769e15b. Integrated main: 29d3dcb7ea85b3be37cd81e977ae4e09cebdd2fc. Linux x64, Node.js 24.18.0/npm 11.16.0, real Chrome 138.0.7204.157.

Redirect resource-type boundary suite: 6 passed, including Fetch/Script negative controls and document exclusion/final-capture races.
npm run check:fast: passed.
Exact-commit built CLI and actual stdio MCP: passed. An owned localhost server serves /app on 127.0.0.1, then returns HTTP 302 to /outside on excluded localhost. Both adapters return target_left_scope, final_url: null, and a redirect event whose excluded URL remains omitted. An independent browser URL confirms the tab reached /outside.
The original before-fix real-browser proof (6e25cc3) returned the stale approved /app URL through both adapters. The historical proof is retained; the runtime fix is now supplied by main.

Integrated upstream EVM cleanup-ownership and PCAP exact-output/fixture-cleanup repairs (07540cbcd, 1341e8c83) that caused the latest CI failures on the companion branches. Their shared source passed the focused EVM/PCAP suites on the same integrated main; this is boundary coverage, not a claim of real EVM/PCAP provider verification.

Verification limits

Current-head CI run 38528150551 passed, including Source tests, Static checks, Build package, all five test shards, Portable tests and CI required. Conventional PR title and Greptile Review also passed. The current head is mergeable. Earlier-head checks remain historical evidence. The local Chrome 138 full-browser lane has an isolated-context origin/storage-seeding limitation; the targeted redirect proof above passes. Other operating systems and Chrome versions were not exercised locally. CI gates and assertions have not been weakened.

_{🔄 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/2885 **Author:** [@zhangguiping-xydt](https://github.com/zhangguiping-xydt) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/browser-redirect-final-url` --- ### 📝 Commits (4) - [`fd815a9`](https://github.com/morluto/rea/commit/fd815a9f491afc5a37fd957dfbc757ce33202321) fix(browser): clear the final URL after an excluded redirect - [`0c43cb9`](https://github.com/morluto/rea/commit/0c43cb91027e04dc3cef92f1d781ae22224c4a21) Merge remote-tracking branch 'origin/main' into HEAD - [`127fa75`](https://github.com/morluto/rea/commit/127fa75c039e1b4301dcbe3234c2faaf349e9dec) chore(browser): integrate upstream CI repairs for redirect observation - [`9b4e5f4`](https://github.com/morluto/rea/commit/9b4e5f43a3b0d3400b1baa6fa660f53ff769e15b) test(browser): retain redirect scope coverage with upstream fixes ### 📊 Changes **1 file changed** (+1 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/browser/cdpObservationResourceTypes.test.ts` (+1 -0) </details> ### 📄 Description ## What Problem This Solves Refs #2593. Browser observation must distinguish a main-frame redirect leaving the approved origins from a cross-origin subresource redirect. The former makes the final page URL unknown; Fetch/Script redirects must preserve the approved page URL. ## Why This Change Was Made Main now includes the original runtime correction in `b98be5408`: `excludeTargetScope()` records exclusion, clears `finalUrl`, and prevents later events or final-frame capture from restoring a stale approved URL. This branch incorporates that implementation and its document-redirect assertions instead of keeping a duplicate assignment. The final diff against main adds one assertion to the existing Fetch/Script negative controls: `target.final_url` must equal `target.initial_url`. Main-frame exclusion and omitted resource-type cases remain covered by upstream tests. Existing maintainer and PR history is preserved. ## User Impact Protects the existing `observe-web-session` and `observe_web_session` behavior: report unknown final locations after scope escape while retaining confirmed page locations after unrelated subresource redirects. No new runtime, schema, dependency, authority or release changes. ## Evidence Current commit: `9b4e5f43a3b0d3400b1baa6fa660f53ff769e15b`. Integrated main: `29d3dcb7ea85b3be37cd81e977ae4e09cebdd2fc`. Linux x64, Node.js 24.18.0/npm 11.16.0, real Chrome 138.0.7204.157. - Redirect resource-type boundary suite: **6 passed**, including Fetch/Script negative controls and document exclusion/final-capture races. - `npm run check:fast`: **passed**. - Exact-commit built CLI and actual stdio MCP: **passed**. An owned localhost server serves `/app` on `127.0.0.1`, then returns HTTP 302 to `/outside` on excluded `localhost`. Both adapters return `target_left_scope`, `final_url: null`, and a redirect event whose excluded URL remains omitted. An independent browser URL confirms the tab reached `/outside`. - The original before-fix real-browser proof (`6e25cc3`) returned the stale approved `/app` URL through both adapters. The historical proof is retained; the runtime fix is now supplied by main. Integrated upstream EVM cleanup-ownership and PCAP exact-output/fixture-cleanup repairs (`07540cbcd`, `1341e8c83`) that caused the latest CI failures on the companion branches. Their shared source passed the focused EVM/PCAP suites on the same integrated main; this is boundary coverage, not a claim of real EVM/PCAP provider verification. ## Verification limits Current-head [CI run 38528150551](https://github.com/morluto/rea/actions/runs/38528150551) **passed**, including Source tests, Static checks, Build package, all five test shards, Portable tests and CI required. Conventional PR title and Greptile Review also passed. The current head is mergeable. Earlier-head checks remain historical evidence. The local Chrome 138 full-browser lane has an isolated-context origin/storage-seeding limitation; the targeted redirect proof above passes. Other operating systems and Chrome versions were not exercised locally. CI gates and assertions have not been weakened. --- <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:53 +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#5468

No description provided.

Rows
Columns