fix(mobile): Harden iOS reconnect, crash capture, and chat layout on main#3910
fix(mobile): Harden iOS reconnect, crash capture, and chat layout on main#3910mwolson wants to merge 11 commits into
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}
ApprovabilityVerdict: Needs human review 5 blocking correctness issues found. This PR introduces substantial new infrastructure for crash logging (native iOS + JS), thread reconnect retry logic, and shell sync healing mechanisms with ~3000 additions. Multiple new files, native code changes, Expo plugins, and 5 unresolved medium-severity review comments identifying potential bugs warrant human review. 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.
CI Mobile Native Static Analysis failed on a multi-line if where the opening brace was on the following line.
Simulator layout evidence (Release Hermes)1. Transparent header: first message not cut offScrolled to the top of a screenshot thread. The first user bubble (prompt + attached screenshot) is fully below the nav chrome; nothing is clipped under the transparent header.
2. Floating composer: content stays above the inputShort screenshot thread after send. Message body and reply sit above the floating composer with clear gap; content is not trapped under the input bar.
All captures: iPhone Simulator, Release + Hermes, |
…x-turn-mapping Squash-merge the full Julian orchestrator v2 stack onto main PRs (ios-fixes-main pingdotgg#3910 and secret-store pingdotgg#2916). Refresh this commit when upstream/t3code/codex-turn-mapping moves; do not cherry-pick its commits individually.
Warm-cache afterSequence resume can skip a dropped thread.archived delta after later events advance snapshotSequence, leaving archived threads on the home list. Always HTTP-heal (or force a full socket snapshot) before resuming live shell deltas, and reject stale full snapshots from the stream.
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 de8b9f6. Configure here.
When HTTP shell heal fails, the full socket snapshot is the membership source of truth. Accept that first snapshot even if its sequence is behind the warm disk cache so ghost active threads cannot stick.
mwolson
changed the title
A prior session can arm acceptNextSocketSnapshotAuthoritatively when HTTP heal fails and then disconnect before the socket snapshot arrives. Clear the flag on a successful HTTP heal so a later reconnect does not treat stale enrichment snapshots as authoritative.
waitForJsonLogMatch only yieldNow'd between attempts, so under busy CI it could return before the ACP mock agent wrote session/cancel. Use a real wall-clock delay (not TestClock sleep) and fail explicitly on timeout.
CI Check treats setTimeout as an Effect lint error. Keep wall-clock delays via a diagnostics-scoped helper, fail with a tagged timeout, and avoid TestClock-bound Effect.sleep so ACP mock log polls still work under load.
Move the header options memo before the missing environmentId, threadId, or selectedThread guards so hook order stays stable if those become null on a re-render of a mounted ThreadRouteContent instance.
1 participant
Summary
main.PreventRemoveProvidervia unstable native-stack options updates.snapshotSequence.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.afterSequenceresume could skip a dropped archive upsert after later shell events advancedsnapshotSequence, so archived threads stayed on Home until a full resync.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.
Focused Simulator layout evidence (two after-state shots only): #3910 (comment)
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)shell-sync): passRemaining Work
Keep exercising heavy-thread catch-up and streaming on a physical iPhone for residual performance issues (memory / large projections are separate from the options-loop fatal).Done: large active CTM thread finished catching up on device after the stack-options fix without thePreventRemoveProviderabort (see Validation).Add focused Simulator layout screenshots.Done in fix(mobile): Harden iOS reconnect, crash capture, and chat layout on main #3910 (comment)Checklist
Note
Harden iOS crash capture, reconnect healing, and thread chat layout
Documents/crash-logsvia a synchronous fsync path; JS errors are intercepted globally and persisted with breadcrumbs viainstallCrashLoggerand a new Expo config plugin (withIosCrashLog.cjs)breadcrumbs.ts) with debounced disk persistence and navigation state recording on every route changeshell.ts: always attempts an HTTP heal on reconnect; if it fails, subscribes withoutafterSequenceso the server sends a full snapshot accepted authoritatively; prevents stale socket snapshots from overriding newer healed stateliveimmediately on reconnect when cached data is present, instead of remainingsynchronizingNativeStackScreenOptionsto callnavigation.setOptionsonly when a structural signature changes, preventing maximum-update-depth crashes from re-entrant header updatesThreadFeed: detects content underflow, auto-aligns short threads to the top anchor, and shows a loading placeholder without remounting the listThreadRouteScreen: retries thread detail load twice with increasing delays, then surfaces a user-facing errorshell.tsreconnect no longer resumes from disk cache alone — every reconnect triggers a server heal, which may increase server load after network interruptionsMacroscope summarized ed1a5f2.
Note
High Risk
Large mobile surface area: native fatal hooks, shell reconnect semantics (more full heals on reconnect), vendored LegendList patches, and Fabric shadow-node layout changes—all affect production iOS chat and sync behavior.
Overview
Adds early crash diagnostics on iOS: breadcrumbs (nav, outbox, thread stop) flushed to
Documents/crash-logs, a JSErrorUtilspath that writes minimal records first then enriches, and nativeRCTSetFatalHandler/ fsync viaT3CrashLogplus an ExpowithIosCrashLogAppDelegate plugin so Release Hermes fatals still leavelast-crash.json.Reconnect and sync: Shell sync no longer resumes
afterSequencefrom disk alone—it HTTP-heals (or takes the first full socket snapshot when HTTP fails), rejects stale full snapshots, and returns shell/thread state to live when cached data exists. Thread detail gets refresh/retry hooks and a stall error after timed retries; sharedcauseFailureMessageimproves error text.Thread UI and layout:
ThreadFeedgains loading overlay, short-content scroll correction, composer gap/inset estimates, and safer screenshot attachments/viewer;LegendList/ keyboard patches handle transparent-header and floating-composer insets. Native markdown rebuilds attributed text inlayout()(fixes missing text) and supportsfillWidth.Stability:
NativeStackScreenOptionsskips redundantsetOptionsvia a structural signature and stable header factories (fixes max update depth on busy threads). Composer uploads strip draft-only fields viatoUploadChatImageAttachments.Reviewed by Cursor Bugbot for commit ed1a5f2. Bugbot is set up for automated code reviews on this repo. Configure here.