Phase 9: check the capture review gate

[krassx]

Summary

• The Phase 9 review gate passes on 2b56960. An unfinished log, network, or breadcrumb filter still drops the line, the pinned deadlines are unchanged, and console dedup still drops the native echo before the user callback.
• Two minors stay out of the fix loop: echo credits can exceed 32 inside one append, and setLogFilter enables the native filter before the JS listener is subscribed. That gap drops the line.

Test plan

• Read-only review of the shipped Phase 9 code at 2b56960
• CI on this plan-only commit

Made with Cursor

[cursor]

Stale comment

Deep review of Phase 9 at 2b56960

This PR only checks the Phase 9 review-gate box. The production surface was reviewed in place at 2b56960, not from this one-line plan diff.

The gate’s core claims hold:

• An unfinished log, network, or breadcrumb filter drops (throw / reject / non-value / never-settles; no JS fail-open timeout).
• Android log and network BORROW_MS is 9_000, under the SDK’s 10s pool recycle; replies after forget do not setMessage / callback.run.
• iOS BGSRNDataRequestDeadlineMs is still 450.
• There is no JS patchXhr or fetch patch.
• Console echo drop runs before the user callback (Android LogFilterBridge.route / iOS BGSRNDropConsoleEcho, then JS classifyFilterRequest).

One deferred item is underclassified. That is enough to keep this gate open.

P2 — setLogFilter enables native before the JS listener exists

Location: packages/react-native/src/logs/filter.ts 36–39 (contrast network/filter.ts 63–71 and breadcrumbs/filter.ts 75–79)

Problem: First setLogFilter calls setLogFilterEnabled(true) and only then onLogFilterRequest(...). Network and breadcrumb already subscribe first. The network comment records that the opposite order dropped the session before stage on a device. Log-filter tests never assert order; breadcrumb tests do.

Impact: Native logcat / RCTLog on other threads during that gap are emitted to nobody. Fail-closed (no secret leak), but the lines are gone. On iOS, emitLogFilterRequest still returns YES when emitOnLogFilterRequest does not throw, so each gap line stays in BGSRNLogFilterPendingTable with no forget timer until process death. Android forgets at 9s.

Scenario: App reaches Launched, calls Bugsee.setLogFilter. Logcat is already live. userFilter / BGSRNLogFilterUserEnabled flips true before the codegen EventEmitter has a JS listener. Adjacent Bugsee.log after return still works (e2e 9.2); concurrent native lines in the window do not.

Fix: Same order as network/breadcrumb: subscribe, then setLogFilterEnabled. Add the breadcrumb-style test (setLogFilterEnabled not called until subscribe returns). Then the gate note can drop this “minor”.

P3 — unbounded echo credits per Android message (agree with the gate)

Location: packages/react-native/android/src/main/java/com/bugsee/reactnative/ConsoleEchoDedup.java 70–74

MAX_MESSAGES caps distinct keys, not credits on one key. note always addLasts. A burst of the same console.log text can hold more than 32 live credits for WINDOW_MS. Extra credits then drop later ReactNativeJS lines of that text. iOS already caps total notes at BGSRNEchoNoteCap (32). Fine to defer; do not treat it as equal to the subscribe hole.

Residuals already accepted — record them on the gate, do not silently close

These are not new blockers (prior Phase 9 reviews accepted them), but a passed gate that lists two “minors” and omits them understates remaining product risk:

• iOS setNetworkFilter replaces the built-in sanitizer; {...event, url} keeps Authorization/cookies (PR 29; JSDoc warns).
• iOS BGSRNOnRCTLog reentrancy flag has no @finally (PR 37).
• Task 9.4’s Android device note (“no network events, no okhttp”) is stale after 9.3 added bugsee-android-okhttp.

Did not run unit/JVM/mutation locally. CI at this review: android, ios unit/spm/cocoapods, and RN compat were green; lint/mutate and ios e2e were still pending.

---

1. Overall risk: Medium
2. Merge recommendation: Request changes. Do not mark Phase 9 passed until setLogFilter subscribes before enable (or the gate stays unchecked with that P2 tracked as a Phase 9 fix, not a minor).
3. Most important to fix: setLogFilter subscribe-then-enable + an order test copied from setBreadcrumbFilter.
4. Positives: Fail-closed is consistent on all three filters; Android 9s borrow vs pool recycle is explicit and tested; native echo drop before the user callback matches design §10.3; Android default network sanitizer still runs before snapshot; patchXhr / JS fetch stay off as required.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Deep review of Phase 9 at a24f167

The prior P2 is fixed. setLogFilter now matches network: subscribe onLogFilterRequest, assign current, then setLogFilterEnabled. current is set before enable, so a synchronous native ask on that turn still has a callback. The new test copies the breadcrumb order assertion (setLogFilterEnabled not called until subscribe returns; later set/clear/set still subscribes once).

That closes the production gap the last review blocked on. Native logcat / RCTLog during first setLogFilter no longer sees userFilter / BGSRNLogFilterUserEnabled before a JS listener exists, so iOS no longer parks those gap lines in BGSRNLogFilterPendingTable with no forget timer.

Gate claims re-checked against HEAD, not only this diff:

• Unfinished log, network, and breadcrumb filters drop (throw / reject / non-value / never-settles). JS does not fail open with the original payload.
• Android log and network BORROW_MS is 9_000, under the SDK’s 10s pool recycle; a reply after forget does not setMessage / callback.run.
• iOS BGSRNDataRequestDeadlineMs is still 450.
• No JS patchXhr or fetch patch.
• Console echo drop still runs before the user callback (Android LogFilterBridge.route / iOS BGSRNDropConsoleEcho, then JS classifyFilterRequest).

No new P0–P2.

Deferred (unchanged, still agreed)

P3 — Android ConsoleEchoDedup.note (ConsoleEchoDedup.java 70–74): MAX_MESSAGES caps distinct keys, not credits on one key. A burst of the same console.log text can hold more than 32 live credits for WINDOW_MS. Extra credits then drop later ReactNativeJS lines of that text. iOS already caps total notes at BGSRNEchoNoteCap (32). Fine to leave on the gate note.

Residuals already accepted — not blockers

• iOS setNetworkFilter replaces the built-in sanitizer; {...event, url} keeps Authorization/cookies (PR 29; JSDoc warns).
• iOS BGSRNOnRCTLog reentrancy flag has no @finally (PR 37).
• Task 9.4’s Android device note (“no network events, no okhttp”) is still stale after 9.3 added bugsee-android-okhttp.

Did not run unit/JVM/mutation locally (no node_modules in this workspace). CI at this review: ios unit/spm/cocoapods and RN compat were green; android, lint/mutate, and ios e2e were still pending.

---

1. Overall risk: Low
2. Merge recommendation: Safe to merge once CI is green. The Phase 9 gate can stay checked.
3. Most important to fix: Nothing blocking. The echo-credit cap remains the only deferred minor.
4. Positives: Subscribe-then-enable is now the same on all three filters, with an order test that would have caught the original hole. Fail-closed is consistent; Android 9s borrow vs pool recycle is explicit and tested; native echo drop before the user callback matches design §10.3; patchXhr / JS fetch stay off.

  

_{Sent by Cursor Automation: Bugsee code review}