fix(mobile): Stabilize the iOS app on main#3910
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size:XXL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Medium
t3code/patches/@legendapp__list@3.2.0.patch
Line 951 in 366f0e0
The onScrollBeginDrag wrapper is created inside a useMemo(..., []) that captures the initial onScrollBeginDrag prop. If a parent passes a new callback after mount, drag events continue invoking the stale closure — the prop update is silently ignored. Store the latest callback in a ref or include onScrollBeginDrag in the memo dependencies.
🤖 Copy this AI Prompt to have your agent fix this:
In file @patches/@legendapp__list@3.2.0.patch around line 951:
The `onScrollBeginDrag` wrapper is created inside a `useMemo(..., [])` that captures the initial `onScrollBeginDrag` prop. If a parent passes a new callback after mount, drag events continue invoking the stale closure — the prop update is silently ignored. Store the latest callback in a ref or include `onScrollBeginDrag` in the memo dependencies.
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
ios-fixes-main
branch
from
366f0e0 to
bfd9ca3
Compare
|
|
||
| let cancelled = false; | ||
| let timer: ReturnType<typeof setTimeout> | null = null; | ||
| const scheduleRetry = () => { |
There was a problem hiding this comment.
🟡 Medium threads/ThreadRouteScreen.tsx:199
The stall error appears after ~40s, not the ~20s implied by THREAD_DETAIL_STALL_ERROR_DELAY_MS. After two failed retries (8s + 12s), scheduleRetry() schedules the error after an additional 20s, so a permanently empty detail doesn't show detailLoadError until roughly 40 seconds. When the retry limit is reached, set the stalled state immediately instead of scheduling another timeout.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadRouteScreen.tsx around line 199:
The stall error appears after ~40s, not the ~20s implied by `THREAD_DETAIL_STALL_ERROR_DELAY_MS`. After two failed retries (8s + 12s), `scheduleRetry()` schedules the error after an additional 20s, so a permanently empty detail doesn't show `detailLoadError` until roughly 40 seconds. When the retry limit is reached, set the stalled state immediately instead of scheduling another timeout.
| void props.listRef.current?.scrollToOffset({ | ||
| animated: false, | ||
| offset: -anchorTopInset, | ||
| }); |
There was a problem hiding this comment.
🟡 Medium threads/ThreadFeed.tsx:1439
When usesNativeAutomaticInsets is false, the underflow correction scrolls to -anchorTopInset, but that same inset is already rendered as the ListHeaderComponent spacer. This double-counts the header inset and positions a short conversation at a negative overscroll offset instead of resting at offset 0. The negative offset should only be used for native automatic-inset layouts where the header inset lives in UIKit's adjustedContentInset and is absent from contentHeight.
| void props.listRef.current?.scrollToOffset({ | |
| animated: false, | |
| offset: -anchorTopInset, | |
| }); | |
| void props.listRef.current?.scrollToOffset({ | |
| animated: false, | |
| offset: usesNativeAutomaticInsets ? -anchorTopInset : 0, | |
| }); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around lines 1439-1442:
When `usesNativeAutomaticInsets` is false, the underflow correction scrolls to `-anchorTopInset`, but that same inset is already rendered as the `ListHeaderComponent` spacer. This double-counts the header inset and positions a short conversation at a negative overscroll offset instead of resting at offset `0`. The negative offset should only be used for native automatic-inset layouts where the header inset lives in UIKit's `adjustedContentInset` and is absent from `contentHeight`.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b2a23fb. Configure here.
ApprovabilityVerdict: Needs human review 6 blocking correctness issues found. This PR introduces a new crash logging infrastructure and makes significant runtime behavior changes to thread loading and navigation. There are unresolved review comments including a high-severity hook ordering issue that could cause crashes when thread data becomes null mid-session. You can customize Macroscope's approvability policy. Learn more. |
Capture RCTFatal message and stack to Documents via a native handler so Release Hermes aborts leave last-crash.json without a device console. Harden the JS ErrorUtils path with a minimal write-first record, durable fsync writes, breadcrumb max-wait flush, and an Expo AppDelegate plugin. Skip NativeStackScreenOptions setOptions when content is unchanged so Thread catch-up re-renders do not loop through PreventRemoveProvider and hit maximum update depth.
| return ""; | ||
| } | ||
| const record = options as Record<string, unknown>; | ||
| return stableJsonStringify({ |
There was a problem hiding this comment.
🟡 Medium native/nativeStackOptionsSignature.ts:12
buildNativeStackOptionsSignature omits fields like headerShown, headerTransparent, presentation, and gestureEnabled. When a caller changes one of these omitted options while the included fields stay the same, the signature is identical, so the skip logic suppresses setOptions and the screen keeps stale options. Consider including all NativeStackNavigationOptions fields in the signature (or documenting why only this subset is tracked).
Also found in 1 other location(s)
apps/mobile/src/native/StackHeader.tsx:114
The signature cache is not scoped to
navigation: when the navigation object/context changes whileprops.optionshas the same signature, this effect reruns but returns at the signature check before calling the new navigator'ssetOptions. The replacement navigator therefore never receives this component's screen options. Reset/cache the signature per navigation object or perform the comparison only after accounting fornavigation.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/native/nativeStackOptionsSignature.ts around line 12:
`buildNativeStackOptionsSignature` omits fields like `headerShown`, `headerTransparent`, `presentation`, and `gestureEnabled`. When a caller changes one of these omitted options while the included fields stay the same, the signature is identical, so the skip logic suppresses `setOptions` and the screen keeps stale options. Consider including all `NativeStackNavigationOptions` fields in the signature (or documenting why only this subset is tracked).
Also found in 1 other location(s):
- apps/mobile/src/native/StackHeader.tsx:114 -- The signature cache is not scoped to `navigation`: when the navigation object/context changes while `props.options` has the same signature, this effect reruns but returns at the signature check before calling the new navigator's `setOptions`. The replacement navigator therefore never receives this component's screen options. Reset/cache the signature per navigation object or perform the comparison only after accounting for `navigation`.
| </View> | ||
| ) : null} | ||
| {props.contentPresentation.kind === "loading" ? ( | ||
| <View pointerEvents="none" className="bg-screen" style={StyleSheet.absoluteFill}> |
There was a problem hiding this comment.
🟡 Medium threads/ThreadFeed.tsx:1890
The loading overlay sets pointerEvents="none", so taps pass through to the still-mounted feed underneath while only the "Loading messages" placeholder is visible. Users can unknowingly activate hidden links, image viewers, or disclosure controls at the tapped coordinates. Consider removing pointerEvents="none" so the overlay intercepts input and blocks interaction with the hidden feed.
| <View pointerEvents="none" className="bg-screen" style={StyleSheet.absoluteFill}> | |
| <View className="bg-screen" style={StyleSheet.absoluteFill}> |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1890:
The loading overlay sets `pointerEvents="none"`, so taps pass through to the still-mounted feed underneath while only the "Loading messages" placeholder is visible. Users can unknowingly activate hidden links, image viewers, or disclosure controls at the tapped coordinates. Consider removing `pointerEvents="none"` so the overlay intercepts input and blocks interaction with the hidden feed.
| </> | ||
| ); | ||
|
|
||
| const threadScreenOptions = useMemo( |
There was a problem hiding this comment.
🟠 High threads/ThreadRouteScreen.tsx:898
The useMemo that builds threadScreenOptions is placed after the early returns for missing environmentId, threadId, or selectedThread. If any of those become null on a re-render (e.g. the thread is removed mid-session), the component returns before reaching useMemo, so React throws "Rendered fewer hooks than expected" and crashes the screen. Move the hook before the conditional returns so the hook count stays constant across renders.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadRouteScreen.tsx around line 898:
The `useMemo` that builds `threadScreenOptions` is placed after the early returns for missing `environmentId`, `threadId`, or `selectedThread`. If any of those become null on a re-render (e.g. the thread is removed mid-session), the component returns before reaching `useMemo`, so React throws "Rendered fewer hooks than expected" and crashes the screen. Move the hook before the conditional returns so the hook count stays constant across renders.
CI Mobile Native Static Analysis failed on a multi-line if where the opening brace was on the following line.
1 participant
Summary
main.PreventRemoveProvidervia unstable native-stack options updates.Problem and Fix
startTurnpayloads.ErrorUtilslogger often lost the race withRCTFatalabort and left empty Documents crash logs.T3NativeControls.writeSyncText), unhandled-rejection capture, nav/outbox/stop breadcrumbs with max-wait flush, and a nativeRCTSetFatalHandlerpath (T3CrashLog+ AppDelegate plugin) that writeslast-crash.json/crash-native-*.jsonwith the real JS message and stack.navigation.setOptionsre-enteredPreventRemoveProvideruntil React hit maximum update depth.setOptions, keep stable header-item factories via refs, and memoize Thread screen options.Defensive Fixes
last-breadcrumbs.jsonnever flushed before a kill.UI Changes
This affects thread loading feedback, screenshot messages and their viewer, transparent-header scrolling, and the floating composer. Before/after evidence and a short interaction recording will be added while the PR remains open.
Validation
vp check: passvp run typecheck: passlast-crash.jsonwithsource: "rct-fatal"and the JS exception message (verified for maximum update depth on Thread)PreventRemoveProviderabortopening_bracefix forT3CrashLog(CI Mobile Native Static Analysis)Remaining Work
Checklist
Note
Stabilize the iOS app with crash logging, scroll fixes, and reconnect recovery
crash-logs/via native sync I/O or Expo FS, and pruned on startup. Navigation state changes are recorded as breadcrumbs.ThreadFeedscroll behavior: initial and underflow states auto-scroll to the correct resting position; the floating composer inset and gap are now correctly accounted for in scroll calculations.StackHeader/NativeStackScreenOptionsto callnavigation.setOptionsonly when a structural signature of options changes, using stable function references for header item factories to avoid unnecessary re-renders.ThreadRouteScreenfor stalled thread-detail loads: retries twice after 8s/12s then surfaces a user-facing error.buildProjectThreadStartTurnInputanduseThreadOutboxDrainto strip client-only fields (id,previewUri) from image attachments before sending.livestatus after reconnect when cached data exists, without requiring new stream events.setReadyin shell and thread state now sets status tolive(notsynchronizing) when a cached snapshot/data is present on reconnect.Macroscope summarized f347d3e.
Note
High Risk
Touches startup crash hooks, AppDelegate prebuild mutation, large LegendList patches, and navigation setOptions behavior—areas that can affect Release stability and scroll correctness across thread screens.
Overview
Backports iOS runtime and chat reliability fixes: durable crash diagnostics, thread feed scroll/composer layout, reconnect and detail-loading behavior, and smaller native/JS hardening.
Crash logging loads first via
installCrashLog, wrapsErrorUtilsand unhandled rejections, writes minimal then full JSON toDocuments/crash-logs/using nativewriteSyncText(fsync), and records nav/outbox/stop breadcrumbs with debounced disk flush. An Expo plugin patches AppDelegate to hookRCTFatal/ fatal exceptions into the same persistence path when JS never flushes.Thread UI tracks content height and composer overlay, adds a composer gap, corrects short-content underflow vs transparent-header insets (LegendList patch +
contentInsetStartAdjustment), shows a loading overlay while hydrating, fixes screenshot attachments (stable layout,Pressable,overFullScreenviewer), and enablesfillWidthon native markdown. T3MarkdownText rebuilds attributed content on everylayout()instead of caching measure-only state so text does not disappear when Yoga skipsmeasureContent.Navigation skips redundant
setOptionswhen a structural signature of stack options is unchanged (avoids Release “maximum update depth” loops). Thread detail retries stalled loads (8s/12s) then surfaces an error; shell/thread syncsetReadyrestoreslivewhen cached data exists after reconnect.startTurn/ outbox sends strip client-only image fields viatoUploadChatImageAttachments. SharedcauseFailureMessageimproves error text in queries and thread state.Reviewed by Cursor Bugbot for commit f347d3e. Bugbot is set up for automated code reviews on this repo. Configure here.