Sign up
View stargazers13Sign up

fix(mobile): Screens 5.0.0-t3.36 stops an iPhone Duo unfold crash#23151

merged2
duo/bottom-line
duo/screens-t3.35
opened2d agobyJJulius Marminge
5 files changed
View on GitHubTwo-way sync enabledPR last imported 2d ago

Unfolding the iPhone Duo right after opening and closing sheets could abort debug builds. A screen re-parented by the split transition detached its touch handler a second time, and RCTSurfaceTouchHandler asserts on that (RCTTouchHandler attached to another view). Screens 5.0.0-t3.35 attaches and detaches the handler only when that changes something, for legacy screens and for split inspector columns.

The release also takes three upstream fixes:

The layer vendors 5.0.0-t3.36, which is t3.35 plus one fix: the search field observer hook from #23146 now stores NSNotificationCenter's original implementation before installing itself. A registration on another thread in between used to call through a null pointer.

Sources: t3-v5.0.0-t3.35 and t3-v5.0.0-t3.36, in pingdotgg/react-native-screens#3. The tarball's t3-fork.json names that commit, and every tracked file in the tarball matches it.

Verification

Fold, open and close the Git overview and Thread settings sheets, then unfold, on the Duo simulator:

ScreensRunsAbortsSource
t3.31 (#22979, #22980 heads)42the motion evidence recordings for those PRs
t3.34 (the layer below this one)713 sheet-flow runs (1 abort), 4 crash-probe runs (none)
t3.3580sheet-flow runs

The abort is intermittent, so the counts support the fix rather than prove it. The fix is the guard on the asserted condition itself. Touches inside the sheets still worked in every t3.35 run. The abort before the fix:

Before: the app aborts as the unfold starts

RCTAssert is compiled out of release builds, so this was likely debug-only, but that is not verified. Upstream #4990's bug did not reproduce in this app, which already showed the tab bar on unfolded cold launches.

Implemented with Claude Opus 5.5 in Claude Code, running in T3 Code.

🤖 Generated with Claude Code

Checks

All checks passed
Ggithub-actionsBot2d ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire—5.0 KiB—6.8 KiB✅
CodexThread snapshot wire—3.8 KiB—4.9 KiB✅
CodexLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
CodexLive turn WebSocket decoded—20.9 KiB—29.3 KiB✅
CodexLive turn messages—2—8✅
ClaudeTotal thread wire—5.0 KiB—6.8 KiB✅
ClaudeThread snapshot wire—3.8 KiB—4.9 KiB✅
ClaudeLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
ClaudeLive turn WebSocket decoded—21.2 KiB—29.3 KiB✅
ClaudeLive turn messages—2—8✅

Baseline: unavailable · PR result: c3dafcf · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

MmacroscopeappBot2d ago
Synced from GitHub

Approvability

Verdict: Approved at c3dafcf

Macroscope's review found this PR approvable — This replaces the mobile Screens fork with a focused native bug-fix release and updates the corresponding package metadata and lockfile. The runtime changes are localized to existing screen, touch-handler, header, sheet, search, and tab behavior, with no new capability or product-default change.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot2d ago
Synced from GitHub

Review in Change Stack →

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 16af903e-dd8d-4f0c-b81a-9003f138fbc6
📥 Commits

Reviewing files that changed from the base of the PR and between 6ecbc7e17b04d3245c54d0da9c5e2f4c1e770246 and a59e6da9b5fa1bc5838b753714a32ea406255484.

⛔ Files ignored due to path filters: 1
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing: 4
  • apps/mobile/deps/react-native-screens-5.0.0-t3.34.tgz
  • apps/mobile/deps/react-native-screens-5.0.0-t3.35.tgz
  • apps/mobile/package.json
  • docs/operations/mobile-screens-fork.md

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

The mobile app now uses the react-native-screens t3.35 archive. The operations documentation now links to the t3.35 fork branch.

Changes: Mobile Screens fork update

Layer / File(s)Summary
Update the fork version references
apps/mobile/package.json, docs/operations/mobile-screens-fork.md		The mobile dependency archive and documented fork branch change from t3.34 to t3.35.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#23013 — The issue requests a newer repack of the same fork. This change updates the dependency to t3.35, but the available summary does not establish that it fixes the reported search state bug.

Suggested reviewers: yash-singh1

Merge Risk: ⚪ Minimal · up to a59e6

The t3.35 dependency update has a reproducible documented source and no actionable merge-blocking risk remains.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Title checkPassedThe title clearly describes the main crash fix and uses a valid conventional format. However, it names Screens 5.0.0-t3.36 while the package and documentation changes target 5.0.0-t3.35.
Description checkPassedThe description clearly explains the problem, the dependency change, upstream fixes, source provenance, and focused verification results. It does not use the required Problem, Change, and Scope and ap…
Full details: Description check

Explanation

The description clearly explains the problem, the dependency change, upstream fixes, source provenance, and focused verification results. It does not use the required Problem, Change, and Scope and approval headings, and it does not provide explicit maintainer approval or a small-fix exemption rationale, but the core information is mostly complete.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Added 2 commits

  • fix(mobile): Screens 5.0.0-t3.35 stops an iPhone Duo unfold crashe94ca8a
  • fix(mobile): Screens 5.0.0-t3.36 installs its search observer hook without a racec3dafcf