[krassx]

Summary

• iOS createReport keeps one outstanding report and enforces the SDK limits up front: at most 3 attachments, each at most 3 MiB.
• fulfil writes only for the generation token captured at reserve time. clear makes older tokens stale.
• File size is taken after stringByResolvingSymlinksInPath. {"description":null} clears the description and leaves the summary.

Test plan

• Created-report XCTests on simulator 6FA9B3E8 (23 in the fix round, 0 failures). First full run was 209 tests
• CocoaPods and SPM example builds
• Not an iPhone pass
• Merge after Task 8.2a. This branch does not contain the Android registry (Task 8.2b).

Made with Cursor

[cursor]

Stale comment

Deep code review (PR #21, a05df5a)

iOS createReport is wired through a one-slot registry with reservation tokens, SDK attachment limits enforced at add time, and invalidate clearing the slot so a reload cannot stay busy. The second commit correctly stops a late SDK completion from minting a handle on the next runtime.

The remaining holes are in the other direction of the same overlap, and in when the slot is considered free relative to uploadReport.

Inspected: the Task 8.2c plan, JS CreatedReport / error contract, handler BGSRNReportOps, Android stubs, XCTest coverage, and the public 7.0.0-beta3 headers (createReportWithCompletion:, uploadReport:completion:, BugseeExtendedReport). XCTests were not re-run here.

Findings

Sev	Issue
P1	invalidate unconditionally clears the process-wide slot, so a new runtime that already reserved or fulfilled can lose that report
P1	take frees the slot before uploadReport finishes, so a second createReport is admitted mid-upload (BUSY contract + P5 globals)
P2	Attachment size is enforced after unbounded main-thread decode/read

Overall risk: Medium

The fill/read/update/add-attachment happy path is careful (all-or-nothing patches, owned file bytes, symlink sizing, P5 canary). Reload-vs-create and upload-vs-create are not.

Merge recommendation: Wait

Do not merge until the two P1s are fixed. The P2 should land with them if screenshots-as-base64 are an expected addDataAttachment path.

Most important to fix

1. Identity-check clear (or otherwise own the slot per module), the same way EventBus / report-handler / data-request detach: already do.
2. Keep the slot occupied until the uploadReport completion (handle can still die at take).

Positive

Reservation tokens and the two stale-fulfil tests match a real overlap (createReport vs invalidate on another queue). validatedPatch:error: keeps handler and created-report validation identical. File attachments are copied at add time, and testTheSdkSharesAttributesAcrossExtendedReports is a useful tripwire for lifting P5.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep code review (PR #21, 5f1dfa0)

Re-review after fix(ios): keep the created-report slot until upload finishes.

detachForUpload: / endUpload: close the same-runtime P1: the handle dies at detach, the slot stays taken until the uploadReport: completion, and a second createReport can no longer -init another BugseeExtendedReport mid-copy. The 3 MiB cap is now applied before initWithBase64EncodedString:, file reads are capped with readDataOfLength: after an NSFileTypeRegular check, and testAnOverlongBase64StringIsRejectedBeforeDecoding / testAnUploadInFlightKeepsTheSlot pin both.

The remaining hole is still invalidate → unconditional clear on a process singleton with no owner. Tokens protect fulfil and endUpload: against stale completions; they do not protect a live slot from the dying module.

Inspected: Task 8.2c, JS CreatedReport / E_REPORT_CREATE_BUSY (P5), handler BGSRNReportOps validatedPatch:, Android 8.2b CreatedReports (same unconditional clear on that PR), EventBus/handler/data-request detach: identity checks, and the new registry tests. XCTests were not re-run here.

Findings

Sev	Issue
P1	invalidate unconditionally clears the process-wide slot, so a new runtime that already reserved, fulfilled, or is uploading can lose that report — and clear sets _uploading = NO, which reopens the P5 attribute wipe during reload-vs-upload

Prior P1 (take before uploadReport completion) and P2 (unbounded main-thread decode/read) are fixed.

Overall risk: Medium

The fill / attach / upload happy path on one runtime is now careful. Reload-vs-create and reload-vs-upload are not, and they sit on the same clear the rest of this module already identity-checks everywhere else.

Merge recommendation: Wait

Do not merge until clear is owner-checked (and an in-flight upload keeps _uploading until endUpload: even across a reload).

Most important to fix

1. Identity-check clear (claim owner on attach / first reserve; invalidate drops the slot only if still owner), matching EventBus / report-handler / data-request detach:.
2. On owner change, drop the handle the new JS cannot address, but do not clear _uploading until that generation's endUpload:.

Positive

detachForUpload: + generation-stamped endUpload: is the right shape for P5. Stale-fulfil tests, the over-long base64 pre-check, capped file reads, and testTheSdkSharesAttributesAcrossExtendedReports remain good tripwires.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep code review (PR #21, 81ce130)

Re-review after fix(ios): keep an in-flight created-report upload across clear.

clear no longer drops _uploading, so a reload during uploadReport: cannot -init another BugseeExtendedReport mid-copy. testClearDuringUploadKeepsTheSlotUntilEnd pins that. Prior P1 (take before upload completion) and P2 (unbounded decode/read) stay fixed.

The remaining hole is the other half of the overlap the registry already documents (createReport vs invalidate on another queue): tokens protect a reservation that exists; they do not protect a reserve that has not run yet.

Inspected: Task 8.2c / P5–P7, JS CreatedReport / E_REPORT_CREATE_BUSY, handler BGSRNReportOps, EventBus/handler/data-request detach:, BGSRNRunOnMain, and the created-report XCTests. XCTests were not re-run here.

Findings

Sev	Issue
P1	reserve is inside BGSRNRunOnMain; invalidate → clear cannot cancel that hop, so a reload leaves the process slot taken by a handle no JS holds

Prior thread (invalidate unconditional clear): partially addressed. Upload hold across clear is done. Steal of a reservation the next runtime already made is the same family; this hop is the failure that does not depend on two module instances.

Overall risk: Medium

Fill/read/update/add-attachment and same-runtime upload-vs-create are in good shape. Reload-vs-in-flight-createReport can still wedge E_REPORT_CREATE_BUSY for the life of the process.

Merge recommendation: Wait

Do not merge until a queued createReport cannot re-occupy the slot after clear.

Most important to fix

1. Admit the slot before the async main hop, or skip reserve once this module is invalidated — clear alone cannot see work that has not reserved yet.

Positive

Reservation tokens still stop a late SDK completion from minting a handle on the next slot. Upload generations plus the _uploading hold close the P5 -init wipe for an in-flight uploadReport:. Attachment limits are enforced before decode / with a capped file read. testTheSdkSharesAttributesAcrossExtendedReports remains a useful tripwire for lifting P5.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep code review (PR #21, 2bcf21d)

Re-review after fix(ios): reserve a created report before the main hop.

reserve now runs on the TurboModule queue before BGSRNRunOnMain. invalidate → clear drops that token. The hop checks reservationIsOpen: and skips createReportWithCompletion: when it is gone, so a torn-down runtime cannot -init another BugseeExtendedReport or hold the slot. testClearBeforeTheHopClosesTheReservation pins the other-queue clear. Prior P1s (take before upload completion; unbounded decode/read; upload hold across clear) stay fixed.

Inspected: createReport vs invalidate/clear, reservationIsOpen: / fulfil:reservation: / _uploading, attachment ops, and BGSRNCreatedReportsTests. XCTests were not re-run here; the author reports 12/12 on simulator 6FA9B3E8.

Findings

No remaining P0–P3.

Owner-checked clear is still declined. That is not re-raised: with admit-before-hop, an overlapping clear yields null or E_REPORT_HANDLE_DEAD and the next createReport can admit; it no longer wedges E_REPORT_CREATE_BUSY for the process lifetime.

Overall risk: Low

Merge recommendation: Safe (into feat/phase-8-2a; this branch still does not contain the Android registry)

Most important to fix

Nothing blocking.

Positive

The hop/clear race is closed at the admission point, not only at fulfil. Skipping the SDK call when the token is gone avoids a gratuitous P5 -init. Upload generations and the 3 MiB / 3-attachment caps remain in place.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep code review (PR #21, 2bcf21d)

Re-review after fix(ios): reserve a created report before the main hop.

Reserving on the calling queue before BGSRNRunOnMain closes the queued-reserve wedge: invalidate → clear drops the token, reservationIsOpen: is NO, and the hop does not call createReportWithCompletion:. testClearBeforeTheHopClosesTheReservation pins that. Prior P1 (take before uploadReport completion) and P2 (unbounded decode/read) stay fixed. Upload still holds _uploading across clear.

The complementary hole is still open: clear zeros _creating after the SDK call has already started. That is the same P5 -init wipe the upload path now prevents, but for createReportWithCompletion: instead of uploadReport:.

Inspected: Task 8.2c / P5–P7, JS CreatedReport / E_REPORT_CREATE_BUSY, handler BGSRNReportOps validatedPatch:, EventBus/handler/data-request detach:, BGSRNRunOnMain, the created-report XCTests, and the public 7.0.0-beta3 headers (createReportWithCompletion:, uploadReport:completion:, BugseeExtendedReport). XCTests were not re-run here.

Findings

Sev	Issue
P1	clear frees the slot while createReportWithCompletion: is in flight; the next runtime -inits another BugseeExtendedReport and can wipe the live report’s attributes

Prior thread (reserve inside BGSRNRunOnMain): fully addressed for the pre-SDK-call path. Prior thread (invalidate unconditional clear): not addressed (owner-checked clear still declined). The remaining failure does not depend on attach-before-invalidate — serial 0.87 teardown is enough.

Overall risk: Medium

Fill/read/update/add-attachment and same-runtime upload-vs-create are in good shape. Reload-vs-in-flight-createReport after the SDK call can still corrupt the next report via P5 globals.

Merge recommendation: Wait

Do not merge until an in-flight createReportWithCompletion: keeps the slot until its (possibly stale) fulfil:, the same way an in-flight uploadReport: keeps _uploading until endUpload:.

Most important to fix

1. Hold the creating slot across clear until that reservation’s completion; do not publish a handle the torn-down JS cannot upload.

Positive

Reservation-before-hop plus reservationIsOpen: is the right check for a queued block clear cannot cancel. Upload generations plus the _uploading hold close the P5 wipe for uploadReport:. Attachment limits are enforced before decode / with a capped file read. testTheSdkSharesAttributesAcrossExtendedReports remains a useful tripwire for lifting P5.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep code review (PR #21, a88e5ea)

Re-review after fix(ios): hold the created-report slot until an in-flight create finishes.

beginCreate: is now the hop’s single gate. NO means clear already dropped the token and the hop skips createReportWithCompletion:. YES marks the SDK call started; a later clear sets _abandoned and keeps _creating until that reservation’s fulfil:, which frees the slot and does not publish a handle. testClearDuringCreateKeepsTheSlotUntilFulfil pins it. The torn-down JS cannot receive cr-<n>, and the next runtime cannot -init another BugseeExtendedReport while beta3 is still using the in-flight report’s file-scope attributes.

Prior P1s (take before uploadReport completion; reserve after the main hop; unbounded decode/read; upload hold across clear) stay fixed.

Inspected: Task 8.2c / P5–P7, JS CreatedReport / E_REPORT_CREATE_BUSY, handler validatedPatch: (extraction only), EventBus/handler/data-request detach:, BGSRNRunOnMain, BGSRNCreatedReports state machine (reserve / beginCreate: / fulfil: / detachForUpload: / endUpload: / clear), attachment ops (3 / 3 MiB, symlink resolve, capped read), and the public 7.0.0-beta3 facts already recorded in the plan (createReportWithCompletion: always completes, including nil when not launched). XCTests were not re-run here. GitHub CI ios unit (BugseeRNSupport) and ios (spm) are SUCCESS on this SHA.

Findings

No remaining P0–P3.

Owner-checked clear is still declined (PRRT_kwDOTk5MSM6n8bjF). That is not re-raised: on serial 0.87 teardown there is no live reservation for invalidate to steal, and an overlapping clear on older RN still yields null / E_REPORT_HANDLE_DEAD rather than a process-lifetime E_REPORT_CREATE_BUSY or a second -init while create/upload is in flight.

Overall risk: Low

Merge recommendation: Safe (into feat/phase-8-2a; this branch still does not contain the Android registry)

Most important to fix

Nothing blocking.

Positive

The create path now matches the upload hold: admit before the hop, mark the SDK call in the same lock as the open check, keep the slot until the completion, never publish a handle the torn-down JS cannot upload. Attachment caps still reject before decode / with a 3 MiB+1 file read. testTheSdkSharesAttributesAcrossExtendedReports remains the tripwire for lifting P5.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep code review (PR #21, 7f748a4)

Re-review after three test-only commits on top of a88e5ea (fix(ios): hold the created-report slot until an in-flight create finishes). Production iOS createReport / registry / attachment code is unchanged.

Task 8.2c is the iOS half of createReport: one process slot (P5, beta3 file-scope attributes), SDK attachment limits enforced at add time (P7: 3 × 3 MiB), and invalidate dropping a handle the next runtime cannot address. The JS CreatedReport / error contract and Android stubs are Task 8.2a / 8.2b and are not implemented here.

Inspected against callers and surrounding code, not the diff alone: the Task 8.2c plan and verified beta3 facts (createReportWithCompletion: always completes, including nil when not launched; uploadReport: copies then completes on main); JS CreatedReport, errors.ts (E_REPORT_CREATE_BUSY / E_REPORT_HANDLE_DEAD), NativeBugsee spec, and index.ts JSDoc; handler BGSRNReportOps validatedPatch: extraction (behavior preserved; created-report ops apply the same dictionary through BugseeExtendedReport setters); EventBus / report-handler / data-request detach: identity checks vs this registry’s unconditional clear; BGSRNRunOnMain (inline on main, dispatch_async off it); the full BGSRNCreatedReports state machine (reserve → beginCreate: → fulfil: / _abandoned hold, detachForUpload: / endUpload:, clear); attachment ops (count, empty, 3 MiB char-cap before decode, NSFileTypeRegular + capped readDataOfLength:, symlink resolve, owned copy); and the XCTest + JS suites including the new commits. XCTests and Jest were not re-run in this agent. GitHub CI on this SHA still had lint, typecheck, unit, mutation, ios unit (BugseeRNSupport), and the iOS example jobs pending at review time. Prior SHA a88e5ea had ios unit (BugseeRNSupport) SUCCESS.

Findings

No remaining P0–P3.

Prior P1s stay fixed: handle dies at detachForUpload: while the slot stays taken until endUpload:; reserve runs before the main hop; beginCreate: is the hop’s gate so clear cannot -init a second BugseeExtendedReport while create/upload is in flight; over-long base64 is rejected before decode and file reads stop at 3 MiB+1.

Owner-checked clear is still declined (PRRT_kwDOTk5MSM6n8bjF). Not re-raised: on serial RN 0.87 teardown there is no live reservation for invalidate to steal. On older attach-before-invalidate the overlap is null / E_REPORT_HANDLE_DEAD, not process-lifetime E_REPORT_CREATE_BUSY and not a second -init mid-copy.

The new tests are the right kind of hardening. BGSRNSettleOnce(0, …) removes the deadline as a third waiter into the two-party barrier (that flake is real: dispatch_after can steal go while both callers are parked). JS now pins severity 1 and 5, the exact TypeError/RangeError strings, omitted-mime null, empty path/name, and native E_REPORT_ATTACHMENT_REJECTED wrapping.

Overall risk: Low

Merge recommendation: Safe (into feat/phase-8-2a; this branch still does not contain the Android registry)

Most important to fix

Nothing blocking.

Positive

The create path still matches the upload hold: admit before the hop, mark the SDK call in the same lock as the open check, keep the slot until the completion, never publish a handle the torn-down JS cannot upload. Attachment caps still reject before decode / with a bounded file read. testTheSdkSharesAttributesAcrossExtendedReports remains the tripwire for lifting P5. validatedPatch: keeps handler and created-report validation identical.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep code review (PR #21, 3db1950)

Re-review after test(ios): keep each settle-once window on its own release. Production iOS createReport / registry / attachment code is unchanged since a88e5ea.

Task 8.2c is the iOS half of createReport: one process slot (P5, beta3 file-scope attributes), SDK attachment limits enforced at add time (P7: 3 × 3 MiB), and invalidate dropping a handle the next runtime cannot address. The JS CreatedReport / error contract and Android stubs are Task 8.2a / 8.2b and are not implemented here.

Inspected against callers and surrounding code, not the diff alone: the Task 8.2c plan and verified beta3 facts (createReportWithCompletion: always completes, including nil when not launched; uploadReport: copies then completes on main); JS CreatedReport, errors.ts (E_REPORT_CREATE_BUSY / E_REPORT_HANDLE_DEAD), NativeBugsee spec, and index.ts JSDoc; handler BGSRNReportOps validatedPatch: extraction (behavior preserved; created-report ops apply the same dictionary through BugseeExtendedReport setters); EventBus / report-handler / data-request detach: identity checks vs this registry’s unconditional clear; BGSRNRunOnMain (inline on main, dispatch_async off it); the full BGSRNCreatedReports state machine (reserve → beginCreate: → fulfil: / _abandoned hold, detachForUpload: / endUpload:, clear); attachment ops (count, empty, 3 MiB char-cap before decode, NSFileTypeRegular + capped readDataOfLength:, symlink resolve, owned copy); and the XCTest + JS suites including the new commits. Adversarial checks included Fast Refresh overlapping an in-flight create/upload, a late SDK completion after clear, double upload(), a fourth / oversize attachment, and a TOCTOU grow between stat and read. XCTests and Jest were not re-run in this agent. GitHub CI on this SHA was still queued at review time. Prior SHA a88e5ea had ios unit (BugseeRNSupport) SUCCESS; its red job was lint, typecheck, unit, mutation (the later test commits).

Findings

No remaining P0–P3.

Prior P1s stay fixed: handle dies at detachForUpload: while the slot stays taken until endUpload:; reserve runs before the main hop; beginCreate: is the hop’s gate so clear cannot -init a second BugseeExtendedReport while create/upload is in flight; over-long base64 is rejected before decode and file reads stop at 3 MiB+1.

Owner-checked clear is still declined (PRRT_kwDOTk5MSM6n8bjF). Not re-raised: on serial RN 0.87 teardown there is no live reservation for invalidate to steal. On older attach-before-invalidate the overlap is null / E_REPORT_HANDLE_DEAD, not process-lifetime E_REPORT_CREATE_BUSY and not a second -init mid-copy.

The latest commit is the right flake fix. A shared go semaphore let the faster settle-once caller leave window 0, enter window 1, and consume the other caller’s release, so that caller never arrived (caller 3 timed out on CI). Separate enter/release pairs per window close that. Combined with BGSRNSettleOnce(0, …), the deadline is no longer a third waiter into the two-party barrier.

Overall risk: Low

Merge recommendation: Safe (into feat/phase-8-2a; this branch still does not contain the Android registry)

Most important to fix

Nothing blocking.

Positive

The create path still matches the upload hold: admit before the hop, mark the SDK call in the same lock as the open check, keep the slot until the completion, never publish a handle the torn-down JS cannot upload. Attachment caps still reject before decode / with a bounded file read. testTheSdkSharesAttributesAcrossExtendedReports remains the tripwire for lifting P5. validatedPatch: keeps handler and created-report validation identical.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Deep code review (PR #21, 601eded)

Re-review after Merge main into this branch. Production iOS createReport / registry / attachment code is unchanged since a88e5ea.

Task 8.2c is the iOS half of createReport: one process slot (P5, beta3 file-scope attributes), SDK attachment limits enforced at add time (P7: 3 × 3 MiB), and invalidate dropping a handle the next runtime cannot address. The JS CreatedReport / error contract is Task 8.2a (now on main). Android is still the 8.2a stubs; the Android registry is Task 8.2b.

The merge kept this branch’s registry and took main’s split ExceptionExecutors, the beta3 override_report.plcrash comment, and omitted dialog fields as nil. Android createReport still resolves null with a single method each — no stub/implementation collision.

Inspected against callers and surrounding code, not the diff alone: the Task 8.2c plan and verified beta3 facts (createReportWithCompletion: always completes, including nil when not launched; uploadReport: copies then completes on main); JS CreatedReport, errors.ts (E_REPORT_CREATE_BUSY / E_REPORT_HANDLE_DEAD), NativeBugsee spec, and index.ts JSDoc; handler BGSRNReportOps validatedPatch: extraction (behavior preserved; created-report ops apply the same dictionary through BugseeExtendedReport setters); EventBus / report-handler / data-request detach: identity checks vs this registry’s unconditional clear; BGSRNRunOnMain (inline on main, dispatch_async off it); the full BGSRNCreatedReports state machine (reserve → beginCreate: → fulfil: / _abandoned hold, detachForUpload: / endUpload:, clear); attachment ops (count, empty, 3 MiB char-cap before decode, NSFileTypeRegular + capped readDataOfLength:, symlink resolve, owned copy); merge of BugseeModule.mm / BugseeModule.java against main; and the XCTest + JS suites. Adversarial checks included Fast Refresh overlapping an in-flight create/upload, a late SDK completion after clear, double upload(), a fourth / oversize attachment, a TOCTOU grow between stat and read, and a second createReport during uploadReport:. XCTests and Jest were not re-run in this agent. GitHub CI on this SHA was still pending at review time. Prior SHA 3db1950 had CI SUCCESS.

Findings

No remaining P0–P3.

Prior P1s stay fixed: handle dies at detachForUpload: while the slot stays taken until endUpload:; reserve runs before the main hop; beginCreate: is the hop’s gate so clear cannot -init a second BugseeExtendedReport while create/upload is in flight; over-long base64 is rejected before decode and file reads stop at 3 MiB+1.

Owner-checked clear is still declined (PRRT_kwDOTk5MSM6n8bjF). Not re-raised: on serial RN 0.87 teardown there is no live reservation for invalidate to steal. On older attach-before-invalidate the overlap is null / E_REPORT_HANDLE_DEAD, not process-lifetime E_REPORT_CREATE_BUSY and not a second -init mid-copy.

Overall risk: Low

Merge recommendation: Safe (into main; this branch still does not contain the Android registry)

Most important to fix

Nothing blocking.

Positive

The merge is the right three-way split: iOS registry kept, Android still the 8.2a stubs, exception executors and dialog-nil behavior taken from main. The create path still matches the upload hold: admit before the hop, mark the SDK call in the same lock as the open check, keep the slot until the completion, never publish a handle the torn-down JS cannot upload. Attachment caps still reject before decode / with a bounded file read. testTheSdkSharesAttributesAcrossExtendedReports remains the tripwire for lifting P5. validatedPatch: keeps handler and created-report validation identical.

  

_{Sent by Cursor Automation: Bugsee code review}