Privacy: secure rectangles and view-tree nodes inside a Modal use the Modal's own origin by krassx · Pull Request #48 · bugsee/bugsee-react-native

krassx · 2026-10-05T08:48:30Z

Summary

<BugseeSecure> rectangles and view-tree (managed) nodes inside a <Modal> are now placed by their own surface's display origin. Before, every rectangle was moved by the activity root's origin, so a secure view in a Modal could be served in the wrong place and recorded in the clear. This is the residual P1 from the Phase 6 (PR 14) review. It was approved earlier on an old base, then ported to main and reworked over three review rounds.

Android

Origin: a Modal's DialogRootViewGroup uses its plain locationOnScreen. Fabric measures Modal content from the Modal's own root, with no viewport offset. Only a ReactRoot (the activity root) uses onScreen − viewportOffset.
Served rectangles: each one is clamped to the display. The SDK's hideRects clamps writes to the buffer but not a rectangle's right edge per row (to be raised with the Android SDK).

iOS

Lanes: each Modal, including pageSheet/formSheet, gets its own lane. Its origin is the presented view's origin in the window, read at pull time.
Rounding: lanes use draft PR iOS: keep secure rectangles and the view tree on windows away from the screen's origin #30's CGPoint origin and outward rounding, so iOS: keep secure rectangles and the view tree on windows away from the screen's origin #30 can rebase onto them later.

JS

Surface key: comes from the nearest RCTModalHostView fiber, once per mount, with no native call.
View-tree walk: asks for an origin at most once per Modal. The walk is back to main's timing: 78/26 ms on the WOD_LX1, against 84/21 on main. The first port attempt took 210/170 ms.
Native: watches the surface on publish and applies its origin at pull time.

Fail closed

A lane whose origin is not known yet, the main lane included, is served as one display-bounds rectangle until the origin is read. The cost is about 3 black frames at the very first publish.
A forgotten dialog root makes its lane unknown again.
A new JS runtime drops the old runtime's Modal lanes, so a production reload (expo-updates, CodePush) cannot leave the whole screen masked.

e2e

Ground truth: each secure view is found by its colour in an independent screenshot (uiautomator on Android, simctl io screenshot on iOS), so the check is not circular.
Discriminating cases: a statusBarTranslucent Modal with edge-to-edge off (Android) and a pageSheet (iOS). Both fail the old code: 8128 and 17760 secure-colour pixels leaked. Both pass now, in the report screenshot and in every video frame.

Merge after #47: the iOS blackout e2e needs #47's marker fix (db9d173). With it applied, blackout passes 5/5.

Follow-ups (Minor):

One JS runtime per process: this is the supported shape, since the bridges are single-slot. A second ReactHost would drop the first's Modal lanes. Document it.
Late publish: a publish from an old runtime that arrives after the new runtime's claim fails closed until restart (rare).
Animated Modal dismissal: the snapshot outlives the rectangle (pre-existing).
Per-pull window search on iOS: PR iOS: keep secure rectangles and the view tree on windows away from the screen's origin #30's tracker replaces it.

Test plan

Jest 2347 passed; lint and typecheck clean; mutate:src 95.29; compat 0.81 and 0.87
Android JVM 336; BugseeRNSupport XCTest 264; the raw-message scanner is green
WOD_LX1 AMRJCP4718402860: edge-to-edge off, secure-modal 8/8 (translucent plus video); edge-to-edge on, secure-modal, blackout, secure-component and view-tree all pass
Simulator 6FA9B3E8: secure-modal 6/6 (pageSheet plus video), secure-component, view-tree; blackout 5/5 with Console dedup: one filter call per line on Android; one iOS e2e marker per line #47's marker fix
Reload with a Modal open, on both devices: the next recording is not masked
Review: 2C/5I/5M, then 0/0/6 minors, then N7, each fixed and re-reviewed; the last re-review approved

🤖 Generated with Claude Code

cursor

Stale comment

Deep review (2366e55)

Per-surface origins for <BugseeSecure> and managed-tree nodes inside a Fabric <Modal> are the right fix for the Phase 6 residual: measureInWindow really does stop at ModalHostViewShadowNode, so a single activity-root origin cannot place a Dialog / pageSheet. The discriminating e2e cases (Android statusBarTranslucent with edge-to-edge off, iOS opaque pageSheet) plus colour-based ground truth are the correct way to prove it.

I did not run the suite locally. At review time CI had android, iOS unit/SPM/CocoaPods, and RN compat 0.81–0.87 green; lint, typecheck, unit, mutation and iOS simulator e2e were still pending.

Findings

P2 BugseeModule.java / BugseeModule.mm — claimRuntime/releaseRuntime drop Modal lanes, but publishes are not bound to the claim. A reload that constructs the new module first (the overlap this protocol exists for) can still write the shared store from the old module. That is not only the documented whole-display fail-closed until process death; if Fabric reassigns the same host tag, the old unmount can clear the new runtime’s Modal rectangles and record them in the clear.

P2 BugseeModule.mm BGSRNModalHost — [window viewWithTag:] returns the first view with that integer and this loop does not keep searching the same window when that hit is not RCTModalHostViewComponentView. A UIKit tag equal to the Modal host’s React tag leaves the lane origin-unknown, so a live secure Modal blackouts the whole recording.

No P0/P1. The late-publish residual is called out in the PR body as a follow-up; it is still open in this SHA, and the tag-reuse overwrite is worse than “fails closed until restart.”

Overall risk: Medium

Merge recommendation: Request changes

Gate writes on the current claim (or ignore publishes from a module whose claim is no longer current) on both platforms, and stop using viewWithTag to find the Modal host.

Most important to fix

Bind set/setCoordinates to the runtime claim so an old module cannot recreate or empty a Modal lane after claimRuntime.

Resolve the iOS host by React tag + class (registry or a walk), not the first viewWithTag hit.

Also noted

Stock RN <Modal visible={false}> unmounts children, so a closed Modal does not keep <BugseeSecure> publishing. That fail-closed path is only a problem for hosts that keep Modal children mounted.

View-tree placement still uses the enclosing origin when originForSurface is null (pinned in tests). That is overlay accuracy, not the video mask.

iOS blackout e2e still depends on PR #47’s marker fix if that is not on main yet.

Positives

Fail-closed unknown origins (including main), display-size clamp on Android for the hideRects row-overflow, origin read at pull time, one origin fetch per Modal per walk (back to main’s timing), Dialog vs ReactRoot viewport handling, and reload dropping Modal keys so a dead surface cannot mask the process for good. Tests actually pin the mutations this would have failed.

_{Sent by Cursor Automation: Bugsee code review}

cursor

Stale comment

Deep review (12a1716)

This SHA is fix/modal-secure-origin plus the merge of #47 (noteConsoleEcho void→boolean). The per-surface origin work is unchanged from 2366e55. Both open P2 threads still stand; I re-read the stores, trackers, JS surface/registry/walk, and the discriminating e2e, not just the merge diff.

The design is still the right fix for the Phase 6 residual: Fabric measureInWindow stops at ModalHostViewShadowNode / DialogRootViewGroup, and a single activity-root origin cannot place a Dialog or pageSheet. Fail-closed unknown origins, Android display clamp, pull-time origin reread, and dropping Modal keys on reload are the right invariants. #47 on this branch is what the iOS blackout e2e needed.

CI on this SHA at review time: ios unit (BugseeRNSupport) and RN compat 0.81–0.87 green; android, iOS SPM/CocoaPods, iOS e2e, and lint, typecheck, unit, mutation still pending.

Findings

P2 BugseeModule.java publishSecure / BugseeModule.mm setSecureRectanglesOnSurface — still open (PRRT_kwDOTk5MSM6o90JT).

Location: packages/react-native/android/src/main/java/com/bugsee/reactnative/BugseeModule.java (~306–314); iOS twin BugseeModule.mm setSecureRectanglesOnSurface: (~1099).

Problem: claimRuntime / releaseRuntime drop every non-main lane, but neither publish path (nor the old module’s tracker) checks secureRuntime against the store’s current claim. set / setCoordinates / forgetOrigin still apply.

Impact: The overlap this protocol exists for — new module constructed, old module not yet invalidated — can put a Modal lane back, or clear/forget-origin the new runtime’s lane. Unknown origin fail-closes to the whole display (the PR’s documented N7 residual). Worse: Fabric retags from small integers after a reload, so the old unmount’s empty publish can land on the new host tag and record the new Modal in the clear.

Scenario: Expo-updates / Fast Refresh full reload with a secure <Modal> still mounted. New BugseeModule claims and JS mounts a similar tree (host tag 42 again) and publishes rectangles. Old JS then tears down and clearOwner publishes [] for 42 through the old module; or the old tracker’s dialog detaches and forgetOrigin(42) while hasRectangles(42) is already the new runtime’s set. releaseRuntime(oldClaim) is a no-op, so the write sticks.

Fix: Pass secureRuntime into publishOrLog / setCoordinates / forgetOrigin and ignore writes whose claim is not current (same serialization as releaseRuntime). Add the JVM/XCTest cases: stale non-empty write must not recreate a lane; stale empty write must not clear the new runtime’s lane; a disposed/old tracker must not forgetOrigin a live claim’s lane.

P2 BugseeModule.mm BGSRNModalHost — still open (PRRT_kwDOTk5MSM6o90Je).

Location: packages/react-native/ios/BugseeModule.mm (~302–306).

Problem: [window viewWithTag:surface] returns the first view in that window with that integer. If that view is not RCTModalHostViewComponentView, the loop moves on to the next window and never continues the search in this one.

Impact: The Modal host is sitting later in the same window, so BGSRNModalSurfaceOrigin stays nil. The lane keeps originKnown = NO and is served as the whole display for as long as that <BugseeSecure> is mounted. Recording/screenshot of an otherwise-working secure Modal is a full blackout, not a tight rectangle. Fail-closed, not a leak — but it makes the feature unusable in mixed UIKit apps.

Scenario: Brownfield / native chrome in the same window (UIButton.tag = 12, a container view.tag = 42, …). The Modal host’s React tag happens to be that same integer (Fabric tags restart at small numbers). viewWithTag: hits the UIKit view first, isKindOfClass: fails, the React host is never considered. The pageSheet e2e does not catch this: that app has no colliding native tags. viewWithTag: is also the wrong primitive for Fabric: tags are not a unique window-wide namespace.

Fix: Do not use viewWithTag. Resolve the host by React tag from the Fabric component registry (or a bounded walk that matches both RCTModalHostViewComponentView and view.tag == surface). If the first tagged view is the wrong class, keep searching this window. A test with a dummy UIView.tag == surface in front of the host should still find the host.

No P0/P1. No new findings from the #47 merge; it only changes noteConsoleEcho.

Overall risk: Medium

Merge recommendation: Request changes

Gate writes on the current claim (or ignore publishes from a module whose claim is no longer current) on both platforms, and stop using viewWithTag to find the Modal host.

Most important to fix

Bind set / setCoordinates / forgetOrigin to the runtime claim so an old module cannot recreate, empty, or origin-forget a Modal lane after claimRuntime.

Resolve the iOS host by React tag + class (registry or a walk), not the first viewWithTag hit.

Positives

Fail-closed unknown origins (including main), display-size clamp on Android for the hideRects row-overflow, origin read at pull time, one origin fetch per Modal per walk, Dialog vs ReactRoot viewport handling, and reload dropping Modal keys so a dead surface cannot mask the process for good. Tests pin the mutations this would have failed. The #47 merge is the right dependency for the iOS blackout e2e marker.

_{Sent by Cursor Automation: Bugsee code review}

Fabric measureInWindow is relative to the measured node's nearest RootNodeKind ancestor, so a rectangle inside a <Modal> is relative to the dialog's DialogRootViewGroup, not the activity root. SecureRectangleStore now keeps one lane per surface (raw rects + that root's locationOnScreen - viewportOffset) and serves them merged in surface-key order; the legacy single-surface set/setOrigin keep meaning the main surface. ReactRootOriginTracker watches extra dialog roots, refreshes their origins with the activity root, mirrors the activity origin onto MAIN_SURFACE for manual writers, and keeps and releases each watched root's layout-listener token. Ported from feat/phase-6-modal (9ec12a2, e6e886f) onto main. TDD: SecureRectangleStoreTest's two-surface cases failed against a surface-ignoring stub (expected 2 rectangles, was 1); tracker tests did not compile without RootHandle.surfaceKey / watchNow. Mutation: publishing a watched dialog root with the activity root's origin fails aWatchedSurfacesOriginMovesOnlyItsRectangles (expected 140, was 100); reverted. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

BGSRNSecureRectangles stores each surface's raw rectangles with the origin that places them and serves the lanes merged in surface-key order; the version moves only when the served bytes change, and an origin-only write that leaves the set empty still reports version 1. The legacy single-surface API means BGSRNSecureMainSurface. On iOS a Fabric <Modal> is presentViewController: on the same UIWindow, so it shares the main surface and that window's frame.origin; only Android dialogs use a second key. Ported from feat/phase-6-modal (9ec12a2, e6e886f) onto main. TDD: the two-surface and version-1 XCTests did not compile against main's store (no surface/origin API). Mutation: translating every lane by the main surface's origin fails testTheMainOriginDoesNotMoveAnotherSurfacesRectangle and testAnotherSurfacesOwnOriginMovesOnlyItsRectangle; reverted. BugseeRNSupport: 237 tests, 0 failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

…urface Fabric measureInWindow is relative to the measured node's nearest RootNodeKind ancestor, and a <Modal> is one (an Android Dialog). The spec gains setSecureRectanglesOnSurface, secureSurfaceKey and secureSurfaceOrigin. The registry keeps one union per display and surface and publishes a non-main surface through setSecureRectanglesOnSurface; <BugseeSecure> asks native for the surface key of the root that holds its view (main surface when it has no tag or the answer is not an integer); the view-tree walk places each node by the origin of its own React root, falling back to the request's origin. Android resolves the root from the view tag on the UI thread, watches it, and returns that root's locationOnScreen - viewportOffset. iOS Fabric presents a Modal on the same UIWindow, so it answers the main surface and the hosting window's frame.origin, and refreshes that origin on every pull. New log lines carry a class name only. Ported from feat/phase-6-modal (9ec12a2, e6e886f) onto main. TDD: the per-surface registry tests failed against a surface-ignoring stub (8 of 9); the BugseeSecure surface tests failed 3 of 15 before the lookup; the walk and requests env tests failed to compile / 15 of 15 before the env fields existed. Mutation: placing every node by the request origin fails 3 walk/requests tests; reverted. Stryker on the four changed files: 95.69%, survivors in new code are equivalent (a no-op republish, a null read inside a try). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Scenario secure-modal mounts a white <BugseeSecure> on the activity root and another inside a transparent <Modal>, logs both measureInWindow rectangles and uploads. secure-modal.test.ts asserts both regions are black in the report's screenshot while a control below the main one stays bright, and on Android that the served set covers the main rectangle (from JS, scaled and moved by the activity origin) and the sheet (from uiautomator) within 2 px. Ported from feat/phase-6-modal (9ec12a2); main already had E2E_METRO_HOST and its own metro watchFolders, so neither is re-added. The unused MAIN_LABEL constant is dropped for lint. Device: WOD_LX1 edge-to-edge on and off, iPhone 17 Pro simulator: 1/1 each, mainInner and sheetInner luma 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Review fix round 1 (C1, C2, I1, I2, M1, M3, M4, M5). C1, Android: the origin of a Modal's DialogRootViewGroup is its locationOnScreen alone. Only a ReactRoot (the activity root) has its viewport offset applied by Fabric; the dialog root is a RootView that is not one, and the modal host node's transform is the identity. ReactRootOriginTracker.surfaceOrigin makes the choice, and the JVM dialog test now feeds a -51 viewport (a translucent status bar) and expects locationOnScreen. C2: no native call per node, and none on a timer. JS reads a view's surface off the fiber tree: the React tag of the nearest RCTModalHostView host above it, or the main surface (src/secure/surface.ts). <BugseeSecure> reads it once per mount and publishes {surface, rects}. The vh walk carries an origin down the tree and asks secureSurfaceOrigin once per Modal per walk. The spec loses secureSurfaceKey. Android watches a Modal's dialog root when JS first publishes on it (found from the host's first child), retries on every refresh until the root exists, and answers secureSurfaceOrigin from that cache. The activity root is the main surface and is watched once (M5). I2, fail closed: a surface other than the main one whose origin has not been read serves one full-display rectangle on both platforms. A Modal whose host tag cannot be read publishes on surface -1, never the main one. Nothing falls back to the main surface on a timeout. I1, iOS: each Modal has its own lane. Its origin is the presented view controller's view in window coordinates plus the window's frame.origin, read at pull time, so a pageSheet is placed inside the window. Per lane the store takes #30's semantics: a CGPoint origin, edges rounded outward and saturating, and re-recording the same origin costs nothing (M3). The comments M4 found deleted are back. M1: an empty Modal lane is dropped when its dialog root detaches (Android) or its host is gone (iOS). Tests: Jest 2093 passed (surface.test.ts, the BugseeSecure, registry, walk and requests surface cases); JVM 287 root and 317 example-Gradle; BugseeRNSupport XCTests 246. Mutation: applying the activity formula to every root fails aDialogRootsOriginIsItsLocationOnScreenAlone and aWatchedSurfacesOriginMovesOnlyItsRectangles; on the WOD_LX1 (edge-to-edge off) the same build fails secure-modal-translucent: 8128 cyan pixels of the Modal's secure view in the report, served sheet 611-811 against 560-760 on screen. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

…ating Modals Review fix round 1 (C1, I1, I3). The secure-modal* scenarios paint the main secure view #FF00FF and the Modal's #00FFFF. Ground truth no longer comes from what the app measured: the device's own screenshot (adb screencap, simctl io screenshot) finds where each colour really is, the report's screenshot must be dark there, and it must not contain either colour anywhere (on the iPhone, which has no command-line screenshot, only the second check runs). New scenarios: secure-modal-translucent (Android; statusBarTranslucent, so with edge-to-edge off the dialog's origin is 0 while the activity root's is the status bar) and secure-modal-sheet (iOS; an opaque pageSheet, inset inside the window). The Android main-rectangle lookup matches the activity root's `kind=activity` origin line instead of `surface=0`, so it cannot be built from the Modal's origin (I3). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

On an iPhone the pageSheet covers the presenting view, so the device screenshot shows only the Modal's colour. The sheet case now requires the main colour to be absent from the device screenshot and checks the report over the sheet alone; the bright control sits inside the Modal's white backdrop below its secure view in every case. Simulator: secure-modal 2/2 and secure-modal-sheet 2/2, sheet found at 104,342-304,442 pt (62 pt below its measureInWindow), sheetInner 0, no secure colour in the report. With every Modal placed at the window origin (the pre-I1 rule) the sheet case fails: 17760 cyan pixels, sheetInner 114.9. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

PR 45 (0eb3cf0) made <BugseeSecure> warn with the error's name rather than the error itself. The surface case added on this branch now expects the same shape. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

…closed on every lane Re-review fix round 2 (N1-N4). N1: a lane with an unknown origin serves the display's real bounds, not a 2^20 square. Android records Display.getRealSize from the root's display on every origin read and from DisplayManager on the first publish; iOS records UIScreen.mainScreen.bounds on every pull. Android also clamps every served rectangle to the display and drops one left with no area: the SDK's native video mask (jni/video.c hideRects) clamps rows to the frame but not a rectangle's right edge to a row, so an oversized one blacked out the start of the next row on every row. Before the size is known the fallback is a 16384 square. N2 (Android): a dialog root the tracker forgets (seen detached on a refresh, or dropped by detach() on host destroy) while its surface holds rectangles has its origin reset to unknown, so it fails closed, and goes back to pending, so a later refresh re-watches it. No origin is recorded from a root that is not attached to its window. N3 (Android): only a non-empty publish watches a surface, and a pending surface whose rectangles JS has cleared is dropped with its empty lane instead of being retried. N4 (ruling): the main lane fails closed too. Until its origin has been read once, its rectangles are served as one display-bounds rectangle. iOS: a note at BGSRNModalSurfaceOrigin for #30's rebase. Tests: JVM root and example-Gradle 332 tests, 0 failures; BugseeRNSupport 261, 0 failures. Mutations, each reverted: main lane not failing closed (13 JVM failures, 4 XCTest), no clamp (everyServedRectangleIsClampedToTheDisplay), no forgetOrigin (2), recording an unattached root's origin (2), no pending prune (2), iOS screen size ignored (4). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Re-review fix round 2 (N6). The Modal holds a red witness view, not secure. On targets with a device screenshot, the test finds the witness there, crops it and the Modal's secure view from every interior video frame (letterbox mapped and checked against video.aux on Android, width-fit on iOS, as secure-component.test.ts does), and requires the secure view to be dark in every frame that shows the witness, with the Modal still up in the last frame. Both crops come from the same frames, so the video's timestamp lag does not matter. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Re-review 2 fix (N7). After a JS reload with a secure Modal open, the old runtime's Modal lane failed closed for the rest of the process: dispose forgot its root and nothing retried it, so every recording was black. A Modal surface's key is a React tag only its runtime knows, so the store now drops every surface but the main one when a runtime goes: the new module claims the store on construction (Android constructor, iOS init) and the old one releases it on invalidate. Claim and release are serialised, and a release by a module whose claim is no longer current drops nothing, so an old module invalidated after the new one started cannot drop the new runtime's surfaces. A disposed Android tracker keeps nothing pending. The main lane keeps the N4 behaviour. N8 (ruled): the store headers now say the main surface stays one whole-display rectangle while no React root can be found. Tests: JVM claimingANewRuntimeDropsEveryModalSurfaceButKeepsTheMainOne, releasingTheCurrentRuntimeDropsItsModalSurfaces, releasingAnOldRuntimeLeavesTheNewRuntimesSurfaces, disposingWithAnOpenModalLeavesNothingPending, and the three XCTest twins. Mutations, each reverted: claim without dropping (2 failures), release ignoring the claim (1), a disposed tracker re-pending (1). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Two debug-level lines (BugseeRN, numbers only): how many Modal surfaces with rectangles a new runtime's claim found left behind, and how many the released runtime held and left. They are the device evidence for N7: a dev reload on the WOD_LX1 logs `released: 0 ... held, 0 left` then `claimed: dropped 0`, since RN's teardown has already cleared the Modal. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

… claim is stale A reload can construct the new BugseeModule before the old one is invalidated, and Fabric numbers React tags from 1 again. The old module's late publish could put back a Modal lane the new claim dropped, or its unmount's empty publish could clear the new runtime's lane on the same tag and uncover the new Modal. Every lane write a runtime makes now carries its claim: the module's publish (publishForRuntime) and its tracker's setOrigin, forgetOrigin and dropSurfaceIfEmpty. The store ignores a write whose claim is not current, under the same lock as claimRuntime/releaseRuntime, and logs it at debug with the two claim numbers only. Addresses PR 48 review thread 4182289966. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

…im is stale The iOS twin of the Android fix. The module's rectangle writes now carry its claim (setCoordinates:count:forDisplay:surface:runtime:). The store ignores one whose claim is no longer current, under the same lock as claimRuntime/releaseRuntime, and logs it at debug with the two claim numbers only. So the old runtime's late publish can neither put back a dropped Modal lane nor clear the new runtime's lane on the same React tag. Origins, display sizes and empty-lane drops come from the wrapper's pull, which reads the live view tree and belongs to no runtime, so they are not claim-gated. Addresses PR 48 review thread 4182289966. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

viewWithTag: returns the first view in a window with that tag. A native view with the same small integer ahead of the RCTModalHostViewComponentView made the lookup skip the rest of that window, so the Modal's origin was never read and its lane failed closed over the whole screen. BGSRNTaggedView walks each window breadth-first, bounded per window, and matches a view that is both the host class and has the tag; a wrong-class view with the tag does not end the search. The Fabric registry is not used: a module reaches it only through viewRegistry_DEPRECATED, and the lookup runs in the wrapper's pull, which has no module. The read stays on main, behind the existing weak host cache. Addresses PR 48 review thread 4182289985. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

cursor

Stale comment

Deep review (f0a421b)

The two open P2s from 12a1716 are fixed in this SHA.

Stale writes (756d531 / 3cbce1b): every runtime lane write carries the claim and is ignored under the same lock as claimRuntime / releaseRuntime. JVM and XCTest pin both cases (stale non-empty does not recreate a dropped lane; stale empty on a reused host tag does not clear the new lane).

viewWithTag: (f0a421b): BGSRNTaggedView walks each window, matches class and tag, and keeps going after a wrong-class hit. The XCTest with a dummy UIView.tag == surface in front of the host is the right pin.

The design is still the right fix for the Phase 6 residual: Fabric measureInWindow stops at ModalHostViewShadowNode / DialogRootViewGroup, so a single activity-root origin cannot place a Dialog or pageSheet. Fail-closed unknown origins, Android display clamp, pull-time origin reread, dropping Modal keys on reload, and the discriminating e2e (translucent Dialog, opaque pageSheet, colour ground truth, video witness) remain the right invariants.

I did not run the suite locally. CI on this SHA at review time: lint, typecheck, unit and RN compat 0.81–0.87 green; android, iOS unit/SPM/CocoaPods, iOS e2e, and mutation still pending.

Findings

P2 BugseeModule.mm BGSRNModalHost — origin lookup is still a process-wide first match after writes were bound to the claim.

Location: packages/react-native/ios/BugseeModule.mm (cache at ~304–310; pull setOrigin: at ~129).

Problem: Rectangle publishes are claim-gated; origin is not. The weak cache returns any live view with that tag, and the walk returns the first RCTModalHostViewComponentView with that tag. claimRuntime does not clear the table.

Impact: Tag reuse during reload overlap binds the new lane to the old host's presented-view origin. That is a misplacement, not fail-closed: the new Modal is recorded in the clear until the old view goes away.

Scenario: Full reload with a secure pageSheet still mounted; new runtime mounts another Modal that Fabric tags 42 again; old host 42 is still in the window. Android is fine (UIManager.resolveView on that module + setOriginForRuntime).

Fix: Resolve the host at publish time from the module (Fabric registry), keep a weak pointer on the lane like Android's watchSurface, and read that at pull. Clear it on claim. Tests: two hosts, same tag, only the second presented.

P3 SecureRectangleStore.dropNonMainSurfaces / iOS dropNonMainSurfacesLocked — main-surface leftovers survive a reload that never republishes on surface 0.

Location: packages/react-native/android/src/main/java/com/bugsee/reactnative/SecureRectangleStore.java (~459–472); iOS twin in BGSRNSecureRectangles.m.

Problem: Claim drops Modal lanes and then ignores stale writes, including []. Main raw is kept, so the old unmount can no longer clear it.

Impact: Persistent extra redaction where the old main <BugseeSecure> was, until process death. Not a leak.

Scenario: expo-updates bundle that removes or does not remount a main-surface secure view. Fast Refresh (same claim) is unaffected.

Fix: Empty the main lane's raw on claimRuntime (keep origin if you still want N4/N8). Do not re-allow stale empty writes on main; that would uncover a new runtime that already published.

No P0/P1.

Overall risk: Medium

Merge recommendation: Request changes

Bind iOS Modal origin to the current runtime's host the way writes (and Android origins) already are. The main-lane leftover can land as a follow-up if you want this in sooner, but it is a real sticky-mask after an OTA that drops main <BugseeSecure>.

Most important to fix

Stop using a process-wide tag walk/cache as the source of truth for a Modal origin after claimRuntime.

Decide what claimRuntime does to the main lane's raw rectangles, now that stale [] cannot clear them.

Positives

The previous P2s were fixed in the shape asked for, with tests that fail if the checks are removed. Claim-gating under the same lock as claim/release, bounded class+tag walk, fail-closed unknown origins (including main), Android hideRects clamp, Dialog vs ReactRoot viewport, one origin fetch per Modal per walk, and colour-based e2e remain the right work.

_{Sent by Cursor Automation: Bugsee code review}

Once stale writes are ignored, the old runtime's unmount [] cannot clear the main set, so a new tree that never publishes on the main surface kept masking where the old <BugseeSecure> was until the process died. claimRuntime now empties the main lane's rectangles and keeps its origin, so the new runtime's first publish is the only main set. Stale empty writes stay ignored. Addresses PR 48 review thread 4184201603. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

The iOS twin of the Android fix: claimRuntime empties the main lane's rectangles and keeps its origin, so the old runtime's main set cannot outlive it when its clearing write is ignored as stale and the new tree never publishes on the main surface. Stale empty writes stay ignored. Addresses PR 48 review thread 4184201603. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

The pull found a Modal's origin by searching the windows for a host with the lane's React tag, and cached the first hit. During a reload Fabric numbers tags from 1 again, so the old runtime's still-presented host can share the tag, and its origin (a sheet inset, say) would place the new runtime's rectangles somewhere else: a leak, not a blackout. The module now names its own runtime's lookup with each Modal publish: its viewRegistry_DEPRECATED (RCTViewRegistry, per runtime), claim-gated in the store like every other write. Fabric mounts the host after JS has published inside it, so the pull asks that lookup on main until it finds an RCTModalHostViewComponentView with the tag, then holds it weakly and reads the origin from it, the shape of Android's watchSurface. A claim forgets every host and lookup; a surface's go with its last lane. Without a registry, or while it finds nothing, the pull falls back to the bounded class+tag walk, which now gives no origin for several matches or for a match that is not presented: the lane stays unknown and is served as the whole display. The weak host cache is gone. Addresses PR 48 review thread 4184201590. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Fabric mounts a Modal's host after the publish that names this runtime's lookup, so the lookup finds nothing for the first pulls of every new Modal. The pull then fell back to the class+tag walk, and during a reload the old runtime's still-presented host can be its only match: for about a second its origin placed the new runtime's rectangles. Once the current runtime has named a host or lookup for a surface (isHostNamedForSurface:), a lookup that finds nothing means the origin is unknown: the lane stays fail-closed, served as the whole display, until the lookup finds this runtime's host. The walk runs only when nothing was ever named, such as without a view registry. Addresses PR 48 review thread 4184201590 (round 2 review, issue 1). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

cursor

Stale comment

Deep review (311de7c)

The two open threads from f0a421b were addressed in this SHA.

Main leftovers (e3463b7 / 86a0b75): claimRuntime now empties the main lane's raw and keeps its origin. JVM/XCTest pin claim + stale [] + no new publish → served empty, and a later publish is placed (not fail-closed).

iOS host binding (311de7c): a Modal publish names this runtime's viewRegistry_DEPRECATED, claim-gated. claimRuntime clears hosts. Two live hosts with the same tag no longer give a first-match origin. That is the right shape, and it matches Android's UIManager.resolveView.

The remaining hole is the walk fallback after a named lookup returns nothing, plus no way to forget a Modal origin once a pull has written one. That is still a misplacement (leak), not fail-closed.

I did not run the suite locally. CI on this SHA was still queued at review time.

Findings

P2 BGSRNSecureSurfaceOrigin — walk fallback can still bind the new lane to the old host (residual of PRRT_kwDOTk5MSM6pCkdH).

See the inline comment. No P0/P1. The P3 main-lane leftover is closed.

Overall risk: Medium

Merge recommendation: Request changes

Do not walk for a surface that already has a current-runtime resolver. When that lookup is empty, stay unknown (Android pending). And when a pull gets no origin for a lane that still has rectangles, forget the previous origin so a one-frame walk hit cannot stick.

Most important to fix

If hostForSurface has a resolver and it has not found the host yet, return no origin — do not call BGSRNUniqueTaggedView.

Add iOS forgetOrigin (or reset originKnown) on that nil path, like Android's detached-root path.

XCTest: old presented host tag 42 + new lane + resolver that still returns nil → origin stays nil; a follow-up pull after a walk once wrote the sheet inset must fail-closed, not keep it.

Positives

Claim-gated writes, emptying main on claim, per-runtime registry lookup, unique-match (not first-match) walk, fail-closed unknown origins, Android display clamp, Dialog vs ReactRoot viewport, one origin fetch per Modal per walk, and the colour/video e2e remain the right work. The new tests actually fail if claim stops clearing main, or if a stale resolver is kept across a claim.

_{Sent by Cursor Automation: Bugsee code review}

cursor

Stale comment

Deep review (a72596f)

The open P2 from 311de7c is fixed.

isHostNamedForSurface now short-circuits BGSRNSecureSurfaceOrigin: once this runtime has named a host or lookup, a miss stays unknown and the lane is served as the whole display. The class+tag walk runs only when nothing was ever named. testAnOldHostAloneWhileTheNewHostIsNotMountedGivesNoOrigin is the case that was missing (old presented tag 42, resolver still nil, walk would have returned the sheet). testAGoneHostTheLookupCannotFindGivesNoOrigin flips the old fallback pin the same way.

That is the Android pending shape. The production path (viewRegistry_DEPRECATED set on publish) no longer places a new lane at the old host during the documented mount-after-publish gap.

Prior threads remain closed: stale writes, viewWithTag:, main-lane empty on claim.

No remaining P0–P3. I did not run the suite locally. CI on this SHA was still queued at review time.

Overall risk: Low

Merge recommendation: Safe once CI is green

Positives

The follow-up is the exact gate asked for, with a test that fails if the walk runs while a lookup is named. Fail-closed unknown origins, claim-gated writes, per-runtime registry, unique-match walk as last resort, Android display clamp, and the colour/video e2e are intact.

_{Sent by Cursor Automation: Bugsee code review}

cursor

Deep review (`a72596f`)

The open P2 from 311de7c is closed in this SHA.

a72596f adds isHostNamedForSurface: and skips BGSRNUniqueTaggedView once this runtime has named a host or lookup. A named lookup that still returns nil (Fabric mounts the Modal host after the publish) now leaves the origin unknown, so the lane is served as the whole display instead of the old runtime's still-presented host. XCTest testAnOldHostAloneWhileTheNewHostIsNotMountedGivesNoOrigin is the case that previously leaked: walk's unique match is the old sheet, resolver is nil, served set is the display bounds. testAGoneHostTheLookupCannotFindGivesNoOrigin no longer encodes the fallback as desired.

I re-read the iOS store/pull, Android tracker/store, JS surface/registry/BugseeSecure/walk, and the colour/video e2e — not just the last commit. No new P0–P3 with a credible failure path.

Prior threads:

Stale writes (PRRT_kwDOTk5MSM6o90JT) — still fully_addressed.
viewWithTag: (PRRT_kwDOTk5MSM6o90Je) — still fully_addressed.
Main leftovers (PRRT_kwDOTk5MSM6pCkdO) — still fully_addressed.
iOS first-match / walk-after-named-nil (PRRT_kwDOTk5MSM6pCkdH, PRRT_kwDOTk5MSM6pDuNe) — fully_addressed (311de7c + a72596f).

I did not run the suite locally. CI on this SHA at review time: lint, typecheck, unit, iOS SPM, and RN compat 0.81/0.83/0.86/0.87 green; android, iOS unit/CocoaPods, iOS e2e, and mutation still pending.

Findings

No remaining P0–P3.

The no-registry class+tag walk is still the fallback when viewRegistry_DEPRECATED is missing. That is the documented path, not the production one this SHA gates. iOS still has no forgetOrigin; after this change a named lookup never writes the old host, so there is nothing stolen to forget on the reload case that was open.

Overall risk: Low

Merge recommendation: Safe

Merge once the remaining CI on a72596f is green (android, BugseeRNSupport, iOS e2e, mutation).

Most important issues to fix

None.

Positives

Per-surface origins are the right fix for Fabric measureInWindow stopping at ModalHostViewShadowNode / DialogRootViewGroup. Claim-gated writes, emptying main on claim, per-runtime host lookup, named-nil fail-closed, unique-match walk, Android display clamp and Dialog vs ReactRoot viewport, one origin fetch per Modal per walk, and the discriminating colour/video e2e (statusBarTranslucent, opaque pageSheet) remain the right work. The new XCTest fails if the walk is allowed to run while a lookup is named.

_{Sent by Cursor Automation: Bugsee code review}

…om the screen's origin JS measures with `measureInWindow`, in the window's points. The iOS SDK that composes every window of the app on the screen (bugsee-cocoa #170-#172, #186) draws secure rectangles and places native view-tree nodes in screen points, so in a window away from the screen's origin (iPad Stage Manager, the right-hand side of Split View, iPhone Duo side by side) a rectangle moved only by the window's `frame.origin` lands off the view it covers, and the view is recorded in the clear. Every lane now takes the window's place on its screen, read the way the SDK places windows (through the screen's fixed space): the main surface, each <Modal> lane from #48 (the presented view's origin in its window plus that place), and the `vh` origin. On a Mac the SDK records the key window's scene alone, and the place stays `frame.origin`. The key window and the windows searched follow the SDK's current pick: the application's key window while its scene is in the foreground, and every foreground scene's windows on that screen when there are several. The SDK pulls the rectangles once per captured frame, and the main surface's origin is re-read on every pull. BGSRNReactRootOriginTracker keeps the React root found last, weakly, and reads its window's place; only without one does a pull search, at most once per 0.5 s (a brownfield app's native screens have no root). A JS publish on the main surface and a `vh` request search at once. Needs the iOS SDK release with that composition; ships with the pin bump. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…e screen's origin (#30) * fix(ios): keep secure rectangles and the view tree on windows away from the screen's origin JS measures with `measureInWindow`, in the window's points. The iOS SDK that composes every window of the app on the screen (bugsee-cocoa #170-#172, #186) draws secure rectangles and places native view-tree nodes in screen points, so in a window away from the screen's origin (iPad Stage Manager, the right-hand side of Split View, iPhone Duo side by side) a rectangle moved only by the window's `frame.origin` lands off the view it covers, and the view is recorded in the clear. Every lane now takes the window's place on its screen, read the way the SDK places windows (through the screen's fixed space): the main surface, each <Modal> lane from #48 (the presented view's origin in its window plus that place), and the `vh` origin. On a Mac the SDK records the key window's scene alone, and the place stays `frame.origin`. The key window and the windows searched follow the SDK's current pick: the application's key window while its scene is in the foreground, and every foreground scene's windows on that screen when there are several. The SDK pulls the rectangles once per captured frame, and the main surface's origin is re-read on every pull. BGSRNReactRootOriginTracker keeps the React root found last, weakly, and reads its window's place; only without one does a pull search, at most once per 0.5 s (a brownfield app's native screens have no root). A JS publish on the main surface and a `vh` request search at once. Needs the iOS SDK release with that composition; ships with the pin bump. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(ios): pin the iOS SDK to 7.0.0-beta4 7.0.0-beta4 is the release that composes every window of the app on the screen (bugsee-cocoa #170-#172, #186), which the previous commit places secure rectangles and the view tree for. - Core: native-versions.json, ios/Support/Package.swift and Package.resolved (revision e467f57c, the bugsee/spm tag). - Feedback: both exact SPM pins and the README. An app using both packages through SPM cannot resolve two different exact pins on bugsee/spm. - The design's Goals and the plan's Global Constraints name the live pin (docs-versions check). - e2e secure-component and secure-modal: from beta4 the iOS SDK writes video.aux version 2. A full-screen app's frame is the screen, in points, with no letterbox padding, so the width-fit mapping still holds; the check now asserts that instead of no video.aux at all. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>