[PR #7204] [MERGED] fix(browser): normalize discovery wire facts at the shared boundary #7714

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7204
Author: @rudycelekli
Created: 10/26/2026
Status: ✅ Merged
Merged: 10/27/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/cdp-socket-target-attached

📝 Commits (6)

d201ce9 fix(browser): keep sibling targets when socket attachment is malformed
78e74b3 fix(browser): reject socket rows without targetId
eaa2ee4 fix(browser): retain valid sibling discovery targets
89854e5 fix(browser): preserve unknown attachment semantics
401f73f fix(browser): scope attachment diagnostics to discovery
4f2c491 Merge latest main and preserve discovery identity ambiguity

📊 Changes

9 files changed (+142 additions, -42 deletions)

View changed files

📝 src/browser/CdpBrowserProvider.ts (+1 -1)
📝 src/browser/CdpDiscoveryTargets.test.ts (+2 -2)
📝 src/browser/CdpDiscoveryTargets.ts (+17 -19)
📝 src/browser/CdpElectronInspection.ts (+1 -1)
📝 src/browser/CdpElectronProvider.ts (+1 -1)
📝 src/browser/CdpEndpoint.test.ts (+49 -0)
📝 src/browser/CdpEndpoint.ts (+9 -17)
📝 src/browser/CdpPageCapture.ts (+1 -1)
📝 tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts (+61 -0)

📄 Description

Summary

Normalize browser discovery at its owning boundary using named HTTP/socket wire schemas. Parse targetId directly instead of copying every raw row into an HTTP-shaped object, and remove the redundant endpoint list schema. Four list/inspection consumers now report attachment as unknown at discovery rather than incorrectly saying a malformed field was omitted.

Main independently landed the basic sibling-preservation behavior and duplicate-ID protection in 4e33bd3e0 while this PR was being verified. This branch has merged main c79bd41b5d6d65b7b2a9b6e0b58d376fc36910d6; it preserves that protection, including IDs repeated in otherwise malformed rows and selection rejection before origin/authority checks. Missing/non-array target lists still fail; explicit empty lists remain valid. No new operation, transport authority or schema surface.

Validation

Refreshed main build and six focused suites: 53 passed, including HTTP/socket ambiguous identities, service-worker authority, endpoint response limits, real CLI inventory, malformed optional fields and socket cleanup. An initial merge-refresh test run exposed a duplicate test import; corrected before the passing run.
Actual Linux Chrome for Testing 156.0.8078.4 with two owned HTTP pages: CLI and MCP match an independent raw-CDP oracle. A fixture-owned transparent proxy injects malformed attachment and two invalid identities; both real pages survive, attachment stays null and source/row exclusions remain explicit. Extended public MCP inspection observes a successful native attach/detach session without rewriting discovery-time unknown attachment. Only injected fields are generated; Chrome did not naturally produce malformed metadata. No native Windows/macOS claim.
Refreshed extended probe: 9.094 s elapsed, 10.148 s CPU, 1678.45 MiB sampled family RSS, two CPUs. Healthy CLI/MCP reply sizes remain 1696/3689 bytes. Earlier alternating baseline/fix probes overlapped in wall time and memory; no stable performance gain/regression is established. /proc RSS sums may double-count shared pages and miss short peaks; model tokens are unmeasured. No new protocol requests or cache.
Serial check:fast: passed in 39.041 s elapsed / 59.654 s CPU, 3460.86 MiB sampled family RSS. The earlier parallel check was stopped at the 4.5 GiB guard, not passed; subsequent checks use supported TURBO_CONCURRENCY=1 without changing CI configuration.

Hosted CI and review

Latest CI 40447062673 tested exact head 4f2c491496c4c929e995b7b26bc31aab0a3ecb66 against main 0eff039d0849951e82b006c719dcfb166b77237a. Static, build, real-browser, web-runtime and Linux native package baseline (including real Node Inspector) passed. Final job totals: 7 successful, 4 failed, 19 skipped and 7 cancelled. The last aggregation-only CI required job completed after merge and failed on the already documented upstream lane failures.

Source suite: 17 failed / 20560 passed / 15 skipped. Unchanged exact-main controls reproduce all 17 failures with matching assertions (17 failed / 49 passed): Apple inventory/provider binding and Windows native loader fixtures.
Shard 8: 21 failed / 2093 passed / 34 skipped. Unchanged exact-main controls reproduce the same 21 failures (21 failed / 65 passed): Electron IPC summary, client configuration/lifecycle and JEB setup selection. No discovery production code participates in these paths.
Seven other test shards were fail-fast cancelled and remain unverified. Portable/required jobs aggregate upstream failures; no claim of all-green CI or native Windows/macOS baseline.

Greptile reviewed exact final 4f2c49149 at 5/5 with no actionable new findings. Its earlier saved-response privacy finding is unchanged upstream code: producer/consumer scope evidence, resolved outside this change, not fixed. Codex could not review because repository quota is exhausted. Readiness declarations remain unchecked; requester explicitly authorized readiness and unrelated-main CI bypass.

Author readiness

Human owner: @rudycelekli

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.

_{🔄 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/7204 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/26/2026 **Status:** ✅ Merged **Merged:** 10/27/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/cdp-socket-target-attached` --- ### 📝 Commits (6) - [`d201ce9`](https://github.com/morluto/rea/commit/d201ce95e6b5ff5a5287f4e92055921d9be3c132) fix(browser): keep sibling targets when socket attachment is malformed - [`78e74b3`](https://github.com/morluto/rea/commit/78e74b311d427b80a57bd91c7a2e1b3fa09c98c0) fix(browser): reject socket rows without targetId - [`eaa2ee4`](https://github.com/morluto/rea/commit/eaa2ee47019caa0ee157fe945934d6f29f086783) fix(browser): retain valid sibling discovery targets - [`89854e5`](https://github.com/morluto/rea/commit/89854e5c40402f97d6b65a03d34f7765d7b3bd8c) fix(browser): preserve unknown attachment semantics - [`401f73f`](https://github.com/morluto/rea/commit/401f73fa0ca9c548fe07de699d2d331476506e69) fix(browser): scope attachment diagnostics to discovery - [`4f2c491`](https://github.com/morluto/rea/commit/4f2c491496c4c929e995b7b26bc31aab0a3ecb66) Merge latest main and preserve discovery identity ambiguity ### 📊 Changes **9 files changed** (+142 additions, -42 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/CdpBrowserProvider.ts` (+1 -1) 📝 `src/browser/CdpDiscoveryTargets.test.ts` (+2 -2) 📝 `src/browser/CdpDiscoveryTargets.ts` (+17 -19) 📝 `src/browser/CdpElectronInspection.ts` (+1 -1) 📝 `src/browser/CdpElectronProvider.ts` (+1 -1) 📝 `src/browser/CdpEndpoint.test.ts` (+49 -0) 📝 `src/browser/CdpEndpoint.ts` (+9 -17) 📝 `src/browser/CdpPageCapture.ts` (+1 -1) 📝 `tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts` (+61 -0) </details> ### 📄 Description ## Summary Normalize browser discovery at its owning boundary using named HTTP/socket wire schemas. Parse `targetId` directly instead of copying every raw row into an HTTP-shaped object, and remove the redundant endpoint list schema. Four list/inspection consumers now report attachment as unknown at discovery rather than incorrectly saying a malformed field was omitted. Main independently landed the basic sibling-preservation behavior and duplicate-ID protection in `4e33bd3e0` while this PR was being verified. This branch has merged main `c79bd41b5d6d65b7b2a9b6e0b58d376fc36910d6`; it preserves that protection, including IDs repeated in otherwise malformed rows and selection rejection before origin/authority checks. Missing/non-array target lists still fail; explicit empty lists remain valid. No new operation, transport authority or schema surface. ## Validation - Refreshed main build and six focused suites: **53 passed**, including HTTP/socket ambiguous identities, service-worker authority, endpoint response limits, real CLI inventory, malformed optional fields and socket cleanup. An initial merge-refresh test run exposed a duplicate test import; corrected before the passing run. - Actual Linux Chrome for Testing 156.0.8078.4 with two owned HTTP pages: CLI and MCP match an independent raw-CDP oracle. A fixture-owned transparent proxy injects malformed attachment and two invalid identities; both real pages survive, attachment stays null and source/row exclusions remain explicit. Extended public MCP inspection observes a successful native attach/detach session without rewriting discovery-time unknown attachment. Only injected fields are generated; Chrome did not naturally produce malformed metadata. No native Windows/macOS claim. - Refreshed extended probe: 9.094 s elapsed, 10.148 s CPU, 1678.45 MiB sampled family RSS, two CPUs. Healthy CLI/MCP reply sizes remain 1696/3689 bytes. Earlier alternating baseline/fix probes overlapped in wall time and memory; no stable performance gain/regression is established. `/proc` RSS sums may double-count shared pages and miss short peaks; model tokens are unmeasured. No new protocol requests or cache. - Serial `check:fast`: passed in 39.041 s elapsed / 59.654 s CPU, 3460.86 MiB sampled family RSS. The earlier parallel check was stopped at the 4.5 GiB guard, not passed; subsequent checks use supported `TURBO_CONCURRENCY=1` without changing CI configuration. ## Hosted CI and review [Latest CI 40447062673](https://github.com/morluto/rea/actions/runs/40447062673) tested exact head `4f2c491496c4c929e995b7b26bc31aab0a3ecb66` against main `0eff039d0849951e82b006c719dcfb166b77237a`. Static, build, real-browser, web-runtime and Linux native package baseline (including real Node Inspector) passed. Final job totals: 7 successful, 4 failed, 19 skipped and 7 cancelled. The last aggregation-only `CI required` job completed after merge and failed on the already documented upstream lane failures. - Source suite: 17 failed / 20560 passed / 15 skipped. Unchanged exact-main controls reproduce all 17 failures with matching assertions (17 failed / 49 passed): Apple inventory/provider binding and Windows native loader fixtures. - Shard 8: 21 failed / 2093 passed / 34 skipped. Unchanged exact-main controls reproduce the same 21 failures (21 failed / 65 passed): Electron IPC summary, client configuration/lifecycle and JEB setup selection. No discovery production code participates in these paths. - Seven other test shards were fail-fast cancelled and remain **unverified**. Portable/required jobs aggregate upstream failures; no claim of all-green CI or native Windows/macOS baseline. Greptile reviewed exact final `4f2c49149` at **5/5 with no actionable new findings**. Its earlier saved-response privacy finding is unchanged upstream code: [producer/consumer scope evidence](https://github.com/morluto/rea/pull/7204#discussion_r4384700954), resolved outside this change, not fixed. Codex could not review because repository quota is exhausted. Readiness declarations remain unchecked; requester explicitly authorized readiness and unrelated-main CI bypass. ## Author readiness Human owner: @rudycelekli - [ ] 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. --- <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:50 +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#7714

No description provided.

Rows
Columns