morluto/rea

Fork 0

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

[PR #612] [MERGED] fix(browser): distinguish duplicate screenshot frame events #2112

New issue

Closed

opened 2026-10-16 21:14:26 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:14:26 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/612
Author: @rudycelekli
Created: 10/5/2026
Status: ✅ Merged
Merged: 10/5/2026
Merged by: @morluto

Base: main ← Head: campaign/rea-screenshot-duplicate-document-20261005

📝 Commits (3)

02fec74 fix(browser): distinguish duplicate screenshot frame events
8fdafff test(browser): preserve unknown with-scope loader bindings
351831f Merge remote-tracking branch 'origin/main' into campaign/rea-screenshot-duplicate-document-20261005

📊 Changes

4 files changed (+67 additions, -3 deletions)

View changed files

📝 src/browser/CdpScreenshot.ts (+24 -2)
➕ tests/boundary/browser/cdpScreenshotDocumentIdentity.test.ts (+29 -0)
📝 tests/fixtures/fakeCdpBrowserEvents.ts (+13 -1)
📝 tests/fixtures/fakeCdpBrowserTypes.ts (+1 -0)

📄 Description

Summary

Use authorized initial URL and nonempty loaderId to ignore identical main-frame document replay during screenshots. Same URL with a new loader still rejects target_changed; preserve other navigation/scope guards. Follow-on to maintainer accepted general capture fix #546, different screenshot path.

No tracking issue: this is a focused correctness regression found through production-seam reproduction.

Problem and expected behavior

Identical authorized URL/loaderId frame events no longer produce a false target_changed outcome; same-URL replacement documents still fail.

Change and scope

The change stays in the screenshot adapter. It preserves the current shared main-frame admission helper and the owner-added child-frame rejection regressions; only identical authorized URL/loader events are ignored.

Contract and boundary impact

Semantic owner and earliest changed stage: Browser CDP screenshot document guard, before retaining screenshot capture output.
CLI and MCP/tool-catalog contract: existing schemas and tool inventory remain intact; corrected results flow through the owning operation.
Provider, bridge, target-format, or platform compatibility: A source-owned loopback CDP WebSocket seam supplied valid document events. No claim is made that Chrome spontaneously emitted duplicate events in this test.
Evidence, artifact, provenance, or reconstruction contract: Identical authorized URL/loaderId frame events no longer produce a false target_changed outcome; same-URL replacement documents still fail.
Process execution, authorization, cleanup, or containment impact: Existing execution authority, cleanup and containment remain preserved.
Generated metadata, package, or installation impact: no metadata or dependency changes.

Evidence and regression coverage

One base-revision false target_changed failure; 19 focused screenshot/provider tests pass, including same-URL/new-loader rejection. This follows the accepted general capture correction in #546 for the separate screenshot path.

Tests exercise production parsing, filesystem, HTTP, or browser seams, without module mocks. Evidence above is executed; unverified platform/provider coverage is stated explicitly.

Validation performed

Passed on signed head 351831f83e5e3c0ff7398c6093d3625564139195 under the current ordinary-fix guidelines:

npm run test:focused -- tests/boundary/browser/cdpScreenshotDocumentIdentity.test.ts tests/boundary/browser/cdpBrowserProvider.document_script.test.ts tests/boundary/browser/webSourceMapFetcher.test.ts — 42 tests passed; boundary build included
npm run check
npm run check:fast

The current main branch, including the maintainer’s fixture correction in #609, was merged normally. Its source-map recovery fixture is preserved exactly: a require inside with remains unknown, while an unbound require outside it and a static import still produce recovered dependencies. This update changes no production code in the PR’s own fix.

Full check:pr is optional for routine fixes under the current guidelines; no current-head full-suite pass is claimed. Package smoke and pack verification are not applicable because package/install outputs and dependencies are unchanged. Hosted CI is tracked separately.

Development runtime: Node.js 24.18.0 and npm 11.16.0.

Compatibility, safety, and release

No migration or package dependency changes. Real Hopper/Ghidra verification was not run; this patch does not change those providers. Remaining proof limits are stated above. The final diff contains source and tests only.

Prepared with AI assistance.

_{🔄 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/612 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `campaign/rea-screenshot-duplicate-document-20261005` --- ### 📝 Commits (3) - [`02fec74`](https://github.com/morluto/rea/commit/02fec74a84f3d169ad8651bb52f8729e5e7882f2) fix(browser): distinguish duplicate screenshot frame events - [`8fdafff`](https://github.com/morluto/rea/commit/8fdafff1fb1d7e556f866dc6e9b8b3d3b7222657) test(browser): preserve unknown with-scope loader bindings - [`351831f`](https://github.com/morluto/rea/commit/351831f83e5e3c0ff7398c6093d3625564139195) Merge remote-tracking branch 'origin/main' into campaign/rea-screenshot-duplicate-document-20261005 ### 📊 Changes **4 files changed** (+67 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/CdpScreenshot.ts` (+24 -2) ➕ `tests/boundary/browser/cdpScreenshotDocumentIdentity.test.ts` (+29 -0) 📝 `tests/fixtures/fakeCdpBrowserEvents.ts` (+13 -1) 📝 `tests/fixtures/fakeCdpBrowserTypes.ts` (+1 -0) </details> ### 📄 Description ## Summary Use authorized initial URL and nonempty loaderId to ignore identical main-frame document replay during screenshots. Same URL with a new loader still rejects target_changed; preserve other navigation/scope guards. Follow-on to maintainer accepted general capture fix #546, different screenshot path. No tracking issue: this is a focused correctness regression found through production-seam reproduction. ## Problem and expected behavior Identical authorized URL/loaderId frame events no longer produce a false target_changed outcome; same-URL replacement documents still fail. ## Change and scope The change stays in the screenshot adapter. It preserves the current shared main-frame admission helper and the owner-added child-frame rejection regressions; only identical authorized URL/loader events are ignored. ## Contract and boundary impact - Semantic owner and earliest changed stage: Browser CDP screenshot document guard, before retaining screenshot capture output. - CLI and MCP/tool-catalog contract: existing schemas and tool inventory remain intact; corrected results flow through the owning operation. - Provider, bridge, target-format, or platform compatibility: A source-owned loopback CDP WebSocket seam supplied valid document events. No claim is made that Chrome spontaneously emitted duplicate events in this test. - Evidence, artifact, provenance, or reconstruction contract: Identical authorized URL/loaderId frame events no longer produce a false target_changed outcome; same-URL replacement documents still fail. - Process execution, authorization, cleanup, or containment impact: Existing execution authority, cleanup and containment remain preserved. - Generated metadata, package, or installation impact: no metadata or dependency changes. ## Evidence and regression coverage One base-revision false target_changed failure; 19 focused screenshot/provider tests pass, including same-URL/new-loader rejection. This follows the accepted general capture correction in #546 for the separate screenshot path. Tests exercise production parsing, filesystem, HTTP, or browser seams, without module mocks. Evidence above is executed; unverified platform/provider coverage is stated explicitly. ## Validation performed Passed on signed head `351831f83e5e3c0ff7398c6093d3625564139195` under the current ordinary-fix guidelines: - `npm run test:focused -- tests/boundary/browser/cdpScreenshotDocumentIdentity.test.ts tests/boundary/browser/cdpBrowserProvider.document_script.test.ts tests/boundary/browser/webSourceMapFetcher.test.ts` — 42 tests passed; boundary build included - `npm run check` - `npm run check:fast` The current main branch, including the maintainer’s fixture correction in #609, was merged normally. Its source-map recovery fixture is preserved exactly: a require inside `with` remains unknown, while an unbound require outside it and a static import still produce recovered dependencies. This update changes no production code in the PR’s own fix. Full `check:pr` is optional for routine fixes under the current guidelines; no current-head full-suite pass is claimed. Package smoke and pack verification are not applicable because package/install outputs and dependencies are unchanged. Hosted CI is tracked separately. Development runtime: Node.js 24.18.0 and npm 11.16.0. ## Compatibility, safety, and release No migration or package dependency changes. Real Hopper/Ghidra verification was not run; this patch does not change those providers. Remaining proof limits are stated above. The final diff contains source and tests only. Prepared with AI assistance. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:14:26 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 00:59:42 +00:00

[PR #2112] [MERGED] docs(ghidra): verified macOS arm64 native decompiler build recipe #5036

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

No description provided.

Rows
Columns