Skip to content

Phase 11: report appearance and delete collected data - #41

Merged
krassx merged 3 commits into
mainfrom
feat/phase-11-appearance
Oct 3, 2026
Merged

krassx merged 3 commits into
mainfrom
feat/phase-11-appearance

Conversation

@krassx

@krassx krassx commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • One camelCase appearance surface maps to Android ReportAppearance (bugsee-android 7.3.0) and the writable report*Color properties on iOS BugseeTheme. An unknown key throws the same RangeError on both platforms.
  • deleteCollectedDataOnDevice(includingIntermediate) passes that boolean through. On iOS the status read, the SDK call, and the missing-completion false run on the main queue.

Test plan

  • Appearance mapping tests (14 keys, unknown key, both booleans) and the Android AppearanceBridgeTest color round-trip
  • WOD_LX1 AMRJCP4718402860 Debug: BUGSEE_E2E appearance nonce=23d750448164 background=#ff0000ff includingIntermediate=true deleted=false
  • iOS Simulator 6FA9B3E8-26C7-4232-AA2C-537D9DF32957 Debug: nonce=96f0670e5a83 with the same color and flag
  • iPhone XS (KRSFT) was already running BareExample and was not run

deleted=false is the SDK refusing deletion while it is still launched. The color was read back on both runs.

Made with Cursor

krassx and others added 2 commits October 3, 2026 06:06
…ata.

The public color names follow bugsee-android 7.3.0 and BugseeTheme in the vendored 7.0.0-beta3 header, and includingIntermediate reaches the SDK's own deletion method.

Co-authored-by: Cursor <cursoragent@cursor.com>
TurboModule calls arrive off main, so the status read and the SDK deletion call follow the same async hop as getStatus. A completion that arrives off main settles there too, once.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Deep code review — Phase 11 appearance and data deletion

The happy path is in good shape: 7.x Android Report:: keys and iOS report*Color properties match the SDK docs, hex is packed ARGB on Android and alpha-last CSS on the way back, and the iOS launched-state missing-completion workaround is a real SDK fact (e2e on both platforms read #ff0000ff back).

The public surface still has two production failures I can construct from this code. Neither is a style nit.

Findings

  1. P2 — Bugsee.appearance getter throws on ordinary reads (other-platform keys, constructor / toString) and cannot be enumerated. (packages/react-native/src/appearance/report.ts)
  2. P2 — iOS deleteCollectedDataOnDevice only papers over a dropped completion when status is not Stopped. The Stopped / never-launched path can hang the promise. (packages/react-native/ios/BugseeModule.mm)
  3. P3 — e2e title claims it records the deletion flag, then never asserts deleted=; the stop-then-delete success path is not run. (examples/bare/e2e/appearance.test.ts)

Overall risk: Medium

Merge recommendation

Do not merge until the two P2s are fixed. Set/get of a shared color after launch works, but the appearance object is unsafe to inspect or dump, and the privacy API’s success path can hang on the same SDK completion bug this PR already special-cases for the launched state.

Most important to fix

  1. Make getters return undefined for a color this platform does not have, and stop intercepting Object.prototype / inspection properties (the feedback package’s defineProperty shape is the existing pattern).
  2. Settle the iOS deletion promise when Stopped too — BGSRNSettleOnce is already used for logUnhandledException on this same SDK build.

Positive

  • Returning a boolean from setAppearanceColor / getAppearanceColor so codegen stays on the JS thread is the right fix for set-then-read in one turn.
  • iOS color keys are whitelisted before KVC, so a bad name cannot NSUnknownKeyException.
  • Android AppearanceBridge round-trips every ReportAppearance color constant and rejects out-of-range components without clobbering a stored value.
  • includingIntermediate is passed through, not reinterpreted.
Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread packages/react-native/src/appearance/report.ts Outdated
Comment thread packages/react-native/ios/BugseeModule.mm
Comment thread examples/bare/e2e/appearance.test.ts Outdated
…letion.

A get of a color this platform does not have was throwing, and a Stopped iOS deletion could wait forever on a completion the SDK never calls. After stop, deletion reports success.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep code review — Phase 11 appearance and data deletion (re-review of c0582c7)

c0582c7 addresses the three issues from the last pass. I re-read the JS appearance object, both native bridges, the TurboModule spec, the feedback-package pattern they copied, Android 7.3.0 / iOS BugseeTheme color lists, and the e2e stop-then-delete path against surrounding lifecycle code. I did not find a new production failure I can construct from this tree.

Prior findings

  1. P2 appearance getter — fully addressed. Colors are enumerable own properties. A get of a name this platform does not have returns undefined without a native call. constructor / toString / then stay on the prototype. Set of an other-platform key still throws. Tests cover all of that.
  2. P2 iOS deletion hang when Stopped — fully addressed. Launched (and Stopping) still resolve false immediately around the missing SDK completion. Stopped, including a nil sharedInstance, goes through BGSRNSettleOnce. Author recorded deleted=true after stop on WOD_LX1 (nonce=87cf6bf967df) and the simulator (nonce=e28a934986ea).
  3. P3 e2e deleted= — fully addressed. The launched run asserts deleted=false. The appearance-stopped run asserts deleted=true.

Findings

None that meet the bar (concrete failure scenario in this code).

I looked at the 1.5s BGSRNUnhandledCompletionDeadlineMs reuse on the Stopped wipe, Android’s lack of a matching hang timeout, and setAppearanceColor returning true without a read-back. None of those has a failure I can show on this SDK: the stop-then-delete e2e already waits 20s and requires deleted=true, Android 7.3.0 does invoke the callback while launched, and appearance set/get of a shared color already round-trips #ff0000ff on both devices.

Overall risk: Low

Merge recommendation

Safe to merge. The public color surface is inspectable, other-platform reads no longer throw, and the privacy API settles on both the launched refusal and the Stopped success path.

Most important to fix

Nothing blocking. Optional later: give the Stopped wipe its own deadline constant so it is not tied to the unhandled-exception 1.5s window.

Positive

  • Boolean setAppearanceColor / getAppearanceColor keeps codegen on the JS thread, so set-then-read in one turn works.
  • iOS keys are whitelisted before KVC; Android packs ARGB and emits CSS #rrggbbaa.
  • The 14 camelCase names match the 7.3.0 ReportAppearance color constants and the writable report*Color properties on BugseeTheme; placeholders stay off this surface.
  • includingIntermediate is passed through, not reinterpreted.
Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

@krassx
krassx merged commit 2b56960 into main Oct 3, 2026
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant