[krassx]

Summary

• upload takes summary, description, severity, and labels. Severity 0 means the SDK default. A fifth argument is refused before the native call.
• showReportDialog calls the no-argument SDK method only when every argument is absent. Otherwise it uses the with-summary form.
• Android maps severity with IssueSeverity.fromIntValue. iOS resolves upload severity 0 from the launch option, or to High.

Test plan

• Jest, ReportArgsTest 6, BGSRNReportArgsTests 6 on simulator 6FA9B3E8, compat 0.81.6, both example builds
• Mutations reverted: ordinal severity, the fifth-argument guard, and unconditional BGSRNUploadSeverity
• No device e2e in this task; that is Task 8.3a, which is blocked on the dialog send control having no stable resource id
• Merge after the Phase 7 PR. Task 8.2a is based on this branch.

Made with Cursor

[cursor]

Stale comment

Deep review (c437703, Task 8.1)

Reviewed against the Phase 8 plan, the existing upload(summary, description) call sites, both native SDKs (Android 7.3.0 AAR + iOS 7.0.0-beta3 headers), and the report-handler path this API feeds. No P0–P3 with a credible failure scenario.

What I checked

• JS contract. Two-arg upload still compiles and crosses severity=0 / labels=null. Explicit 0, 6, 2.5, non-numbers, non-string labels, and a fifth argument are rejected before the bridge; error text does not echo the rejected value. Labels are copied. showReportDialog() crosses null, null, 0, null.
• Severity mapping. JS IssueSeverity.Critical is 4. Android uses IssueSeverity.fromIntValue after a 1..5 guard (the one-arg fromIntValue would otherwise snap unknowns to VeryLow; values()[4] is Blocker). iOS upload resolves 0 through BugseeOptionReportingDefaultBugPriority or High (3); requested 1..5 wins. Tests pin value-not-ordinal and reject a boolean-boxed launch option.
• Android 7.3.0. showReportDialog() is literally four nulls into the same internal method. createBugReportAndUpload / createAndPrepareBugReport skip setSeverity / setLabels when those arguments are null, so omitted fields keep the SDK default. Labels are materialized with toArrayList() on the native-modules thread before the UI hop, so the ReadableArray is not used after the method returns. Void methods catch RuntimeException so an SDK throw cannot kill the host.
• iOS 7.0.0-beta3. Dialog summary/description are nonnull, so nil becomes @"" as specified. Labels are _Nullable. The no-arg selector is used only when every argument is absent; otherwise 0 severity is left for the SDK’s if (level) skip. Upload cannot use the two-arg selector once labels exist, which is why 0 is resolved natively. Both run inside BGSRNRunOnMain. Existing two-arg scenario calls are unchanged.
• Compatibility. TurboModule spec grows upload by two required parameters and adds showReportDialog. ios-spec-coverage derives selectors from the spec (upload:description:severity:labels:, showReportDialog:description:severity:labels:). RN compat 0.81–0.87 is green on this SHA.

I did not run the workspace Jest locally (no node_modules in this agent). CI on this SHA: android, ios cocoapods/spm, ios unit (BugseeRNSupport), ios e2e simulator, and rn compat 0.81–0.87 all passed. lint, typecheck, unit, mutation was still running at review time.

Device proof that severity/labels land in retained bundles is Task 8.3a, which this PR correctly leaves out.

Overall risk: Low

Merge recommendation

Safe to merge into feat/phase-7-5b for this slice. Task 8.2a is based on this branch; nothing here needs to wait on 8.3a.

Most important issues to fix

None for this PR.

Positives

The ordinal trap is treated as load-bearing (Android test literally asserts values()[4] is Blocker while wire 4 is Critical). The includeVideo fifth-argument guard fails 6.x call sites loudly instead of dropping the flag. Native marshalling is unit-tested without RN (ReportArgsTest, BGSRNReportArgsTests), and the facade tests pin copy-on-labels and secret-free error messages.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review (53a2e97, Task 8.1)

Re-review of the two test follow-ups on top of c437703, against the Phase 8 plan, JS callers of two-arg upload, Android 7.3.0 (createAndPrepareBugReport / createBugReportAndUpload), and iOS 7.0.0-beta3 (showReportControllerWithSummary:description:severity:labels:).

One P1. The rest of the slice still matches the plan.

Finding

P1 — iOS showReportDialog turns omitted summary/description into @"", which the SDK treats as a provided field. The no-arg selector exists because nil and empty are not the same; that distinction is then discarded on every partial call. Details on the conversion lines.

What I checked (no additional findings)

• JS contract. Two-arg upload still crosses severity=0 / labels=null. Explicit 0, 6, 2.5, non-numbers, non-string labels, null labels, and a fifth argument are rejected before the bridge; error text does not echo the rejected value. Labels are copied. showReportDialog() crosses null, null, 0, null. showReportDialog('s') correctly crosses description as null — the loss happens natively on iOS only.
• Severity mapping. JS IssueSeverity.Critical is 4. Android uses IssueSeverity.fromIntValue after a 1..5 guard (one-arg fromIntValue snaps unknowns to VeryLow; values()[4] is Blocker). iOS upload resolves 0 through BugseeOptionReportingDefaultBugPriority or High (3); requested 1..5 wins. Boolean-boxed launch options are rejected (CFBooleanGetTypeID).
• Android 7.3.0. showReportDialog() is four nulls into showBugReportingDialog. Coordinator skips setSeverity / setLabels on null; setSummary / setDescription always run (including null), which is the same as the SDK's own no-arg path. Labels are toArrayList() on the native-modules thread before the UI hop. Void methods catch RuntimeException.
• iOS 7.0.0-beta3 (disassembled). No-arg showReportDialog calls showReportController and never seeds fields. The with-summary completion does cbz then setSummary: / setReportDescription: / setSeverity: / clearLabels+addLabels:. Pointer cbz is a nil check (@"" is non-nil). Integer cbz is why severity 0 is skipped, as specified. Upload still resolves 0 because the four-arg selector cannot carry labels otherwise. Both hop with BGSRNRunOnMain (async off main).
• Tests in this SHA. db302b5 pins severity bounds and exact error strings. 53a2e97 drops the settle-once race's deadline so it cannot steal a barrier signal on a slow runner — production BGSRNSettleOnce is unchanged.
• Compatibility. Spec grows upload by two required parameters and adds showReportDialog. Existing two-arg JS call sites still compile. I did not run workspace Jest (no node_modules here). CI on this SHA at review time: android, ios cocoapods, ios unit, ios e2e simulator, rn compat 0.81–0.87 passed; ios spm and lint, typecheck, unit, mutation were still pending.

Device proof that severity/labels land in retained bundles remains Task 8.3a.

Overall risk: Medium

Merge recommendation

Do not merge until the iOS dialog nil→@"" conversion is fixed (or proven equivalent to omit in the dialog UI). The upload path and the JS facade are otherwise ready; Task 8.2a can stay based on this branch after the dialog fix.

Most important issues to fix

1. Pass omitted iOS dialog summary/description as nil, not @"", so the SDK cbz skip matches Android's null and the JS "absent → null" contract.

Positives

The ordinal trap is still load-bearing (Android test asserts values()[4] is Blocker while wire 4 is Critical). The includeVideo fifth-argument guard fails 6.x call sites loudly. Native marshalling is unit-tested without RN. The settle-once test change is the right fix for a third caller into a two-party barrier.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review (924a3bf, Task 8.1)

Re-review of HEAD 924a3bf (per-window settle-once test fix on top of 53a2e97 / c437703), against the Phase 8 plan, JS callers of two-arg upload, Android 7.3.0 (showBugReportingDialog / createAndPrepareBugReport), and the iOS 7.0.0-beta3 dialog contract already recorded in this repo (if (summ), if (level), if (labels)).

The prior P1 is not addressed. 924a3bf only changes BGSRNSettleOnceTests; production showReportDialog still converts omitted iOS text to @"".

Finding

P1 — iOS showReportDialog turns omitted summary/description into @"", which the SDK treats as a provided field. The no-arg selector exists because nil and empty are not the same; that distinction is then discarded on every partial call. Details on the conversion lines. Independently re-checked on this SHA: JS still crosses omitted text as null (optionalReportText); Android still forwards that null; the iOS handler snapshot still keeps an empty NSString as a present string.

What I checked (no additional findings)

• JS contract. Two-arg upload still crosses severity=0 / labels=null. Explicit 0, 6, 2.5, non-numbers, non-string labels, null labels, and a fifth argument are rejected before the bridge; error text does not echo the rejected value. Labels are copied. showReportDialog() crosses null, null, 0, null. showReportDialog('s') correctly crosses description as null — the loss happens natively on iOS only.
• Severity mapping. JS IssueSeverity.Critical is 4. Android uses IssueSeverity.fromIntValue after a 1..5 guard (one-arg fromIntValue snaps unknowns to VeryLow; values()[4] is Blocker). iOS upload resolves 0 through BugseeOptionReportingDefaultBugPriority or High (3); requested 1..5 wins. Boolean-boxed launch options are rejected (CFBooleanGetTypeID). The High fallback matches the 7.x Android options manifest default for bug-priority.
• Android 7.3.0. showReportDialog() is four nulls into showBugReportingDialog. Labels are toArrayList() on the native-modules thread before the UI hop. Void methods catch RuntimeException.
• iOS dialog vs upload. Upload still resolves 0 because the four-arg selector cannot carry labels otherwise. Dialog leaves 0 for the SDK if (level) skip, as specified. Both hop with BGSRNRunOnMain (async off main).
• 924a3bf. Production BGSRNSettleOnce is unchanged. The test now uses one release semaphore per window so a fast caller cannot consume the other caller's go signal. That is the right fix for the CI timeout; it does not affect this slice's production API.
• Compatibility. Spec grows upload by two required parameters and adds showReportDialog. Existing two-arg JS call sites still compile. I did not run workspace Jest or native unit tests in this agent.

Device proof that severity/labels land in retained bundles remains Task 8.3a. That task's dialog case passes every field, so it would not catch this omit bug.

Overall risk: Medium

Merge recommendation

Do not merge until the iOS dialog nil→@"" conversion is fixed (or proven equivalent to omit in the dialog UI and in onBeforeReportCreated). The upload path, the JS facade, and the settle-once test fix are otherwise ready; Task 8.2a can stay based on this branch after the dialog fix.

Most important issues to fix

1. Pass omitted iOS dialog summary/description as nil, not @"", so the SDK if (summ) skip matches Android's null and the JS "absent → null" contract.

Positives

The ordinal trap is still load-bearing (Android test asserts values()[4] is Blocker while wire 4 is Critical). The includeVideo fifth-argument guard fails 6.x call sites loudly. Native marshalling is unit-tested without RN. The settle-once test change is the right fix for a shared release stealing a two-party barrier.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review (a88a7e0, Task 8.1)

Re-review of HEAD a88a7e0 against the Phase 8 plan, two-arg upload callers, Android 7.3.0 (showBugReportingDialog / createAndPrepareBugReport), and the iOS 7.0.0-beta3 dialog contract (if (summ), if (level), if (labels)).

The prior P1 is fully addressed. a88a7e0 passes omitted iOS dialog summary/description through as nil; the no-arg selector is unchanged. Rechecked on this SHA: JS still crosses omitted text as null (optionalReportText); Android still forwards that null; the with-arguments iOS path no longer substitutes @"".

No new P0–P3 with a credible failure scenario.

What I checked

• JS contract. Two-arg upload still crosses severity=0 / labels=null. Explicit 0, 6, 2.5, non-numbers, non-string labels, null labels, and a fifth argument are rejected before the bridge; error text does not echo the rejected value. Labels are copied. showReportDialog() crosses null, null, 0, null. showReportDialog('s') crosses description as null — and native iOS now leaves that nil.
• Prior P1. showReportDialogWithSummary: is called with summary / description as-is. beta3 only writes those fields when the pointer is non-nil (cbz / if (summ)), so nil is omit. @"" remains only when JS actually sent an empty string. Severity 0 and nil labels are unchanged.
• Severity mapping. JS IssueSeverity.Critical is 4. Android uses IssueSeverity.fromIntValue after a 1..5 guard (one-arg fromIntValue snaps unknowns to VeryLow; values()[4] is Blocker). iOS upload resolves 0 through BugseeOptionReportingDefaultBugPriority or High (3); requested 1..5 wins. Boolean-boxed launch options are rejected (CFBooleanGetTypeID). The High fallback matches the 7.x Android options manifest default for bug-priority.
• Android 7.3.0. showReportDialog() is four nulls into showBugReportingDialog. Labels are toArrayList() on the native-modules thread before the UI hop. Void methods catch RuntimeException.
• iOS dialog vs upload. Upload still resolves 0 because the four-arg selector cannot carry labels otherwise. Dialog leaves 0 for the SDK if (level) skip. Both hop with BGSRNRunOnMain (async off main).
• Settle-once tests. Production BGSRNSettleOnce is unchanged. Per-window release semaphores keep the two-party barrier from stealing a signal on a slow runner.
• Compatibility. Spec grows upload by two required parameters and adds showReportDialog. Existing two-arg JS call sites still compile. I did not run workspace Jest or native unit tests in this agent. CI on this SHA at review time: android, ios spm, and rn compat 0.81–0.87 passed; ios cocoapods, ios unit, ios e2e simulator, and lint, typecheck, unit, mutation were still pending.

Device proof that severity/labels land in retained bundles remains Task 8.3a.

Overall risk: Low

Merge recommendation

Safe to merge into feat/phase-7-5b. The dialog omit bug is fixed; the upload path, the JS facade, and the settle-once test fixes were already ready. Task 8.2a can stay based on this branch.

Most important issues to fix

None for this PR.

Positives

The ordinal trap is still load-bearing (Android test asserts values()[4] is Blocker while wire 4 is Critical). The includeVideo fifth-argument guard fails 6.x call sites loudly. Native marshalling is unit-tested without RN. a88a7e0 matches the SDK’s pointer check instead of the plan’s ?: @"", which is the right correction.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Deep review (58dddec, Task 8.1)

Re-review of HEAD 58dddec (Merge main into feat/phase-8-1, parents a88a7e0 + 57af056) against the Phase 8 plan, two-arg upload callers, Android 7.3.0 (showBugReportingDialog / createAndPrepareBugReport), and the iOS 7.0.0-beta3 dialog contract (if (summ), if (level), if (labels)).

The prior P1 remains fully addressed. Omitted iOS dialog summary/description still pass through as nil; the no-arg selector is unchanged. The merge kept main’s split ExceptionExecutors (handled vs unhandled queues) and the beta3 override_report.plcrash recovery comment, and did not revive the @"" substitution.

No new P0–P3 with a credible failure scenario.

What I checked

• Merge resolution. BugseeModule.java still posts handled reports onto ExceptionExecutors.HANDLED and unhandled onto UNHANDLED (the #17 split), then the 8.1 upload / showReportDialog mapping. iOS logUnhandledException still carries main’s beta3 recovery note; showReportDialogWithSummary: still takes summary / description as-is (BugseeModule.mm 647–650). Support/SPM and CocoaPods import branches both pull in BGSRNReportArgs.h.
• Prior P1. beta3 only writes dialog fields when the pointer is non-nil (cbz / if (summ)), so nil is omit. @"" remains only when JS actually sent an empty string. Rechecked: JS optionalReportText still crosses omitted text as null; Android still forwards that null; iOS no longer substitutes @"".
• JS contract. Two-arg upload still crosses severity=0 / labels=null. Explicit 0, 6, 2.5, non-numbers, non-string labels, null labels, and a fifth argument are rejected before the bridge; error text does not echo the rejected value. Labels are copied. showReportDialog() crosses null, null, 0, null. showReportDialog('s') crosses description as null, and native iOS leaves that nil.
• Severity mapping. JS IssueSeverity.Critical is 4. Android uses IssueSeverity.fromIntValue after a 1..5 guard (one-arg fromIntValue snaps unknowns to VeryLow; values()[4] is Blocker). iOS upload resolves 0 through BugseeOptionReportingDefaultBugPriority or High (3); requested 1..5 wins. Boolean-boxed launch options are rejected (CFBooleanGetTypeID). The High fallback matches the 7.x Android options manifest default for bug-priority.
• Android 7.3.0. showReportDialog() is four nulls into showBugReportingDialog. Labels are toArrayList() on the native-modules thread before the UI hop. Void methods catch RuntimeException. Existing two-arg example upload call sites still typecheck.
• iOS dialog vs upload. Upload still resolves 0 because the four-arg selector cannot carry labels otherwise. Dialog leaves 0 for the SDK if (level) skip. Both hop with BGSRNRunOnMain (inline on main, async off main).
• Settle-once tests. Production BGSRNSettleOnce is unchanged. Per-window release semaphores keep the two-party barrier from stealing a signal on a slow runner.
• Compatibility. Spec grows upload by two required parameters and adds showReportDialog. Existing two-arg JS call sites still compile. I did not run workspace Jest or native unit tests in this agent. CI on this merge SHA was still in progress at review time; a88a7e0 was green.

Device proof that severity/labels land in retained bundles remains Task 8.3a.

Overall risk: Low

Merge recommendation

Safe to merge into main. The dialog omit bug stays fixed; the upload path, the JS facade, and the settle-once test fixes were already ready; this merge only folds Phase 7’s executor split and recovery comment without touching that contract.

Most important issues to fix

None for this PR.

Positives

The ordinal trap is still load-bearing (Android test asserts values()[4] is Blocker while wire 4 is Critical). The includeVideo fifth-argument guard fails 6.x call sites loudly. Native marshalling is unit-tested without RN. The merge kept the right side of both conflicts: split exception threads from main, nil dialog fields from this branch.

  

_{Sent by Cursor Automation: Bugsee code review}