Skip to content

fix(mobile): stop crashing on launch before the shell snapshot arrives - #11537

Merged
juliusmarminge merged 1 commit into
mainfrom
fix/project-settings-null-project
Sep 13, 2026
Merged

fix(mobile): stop crashing on launch before the shell snapshot arrives#11537
juliusmarminge merged 1 commit into
mainfrom
fix/project-settings-null-project

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 13, 2026

Copy link
Copy Markdown
Member

Build 1.1.1 (56) still crashed on every cold open even with #11486 in it. TestFlight strips the JS error, but expo-updates writes it to the app container (Library/Application Support/expo-error.log), pulled off the device with devicectl:

Fatal error: Domain: RCTErrorDomain
Description: Unhandled JS Exception: TypeError: Cannot read property 'defaultModelSelection' of null
    at NewTaskFlowProvider (main.jsbundle:590585:51)
    at NavigationProvider …

resolveProjectSettings (added in #11176) reads the project aggregate's legacy defaultModelSelection/defaultThreadEnvMode while projectSettingsFolded is false. The mobile new-task flow (#10639) passes its selectedProject straight through, and that is null until the shell snapshot arrives — so the very first render of NewTaskFlowProvider throws, RN reports a fatal, and expo-updates' ErrorRecovery aborts the process ~1 s after launch. That is the same native trace as the toReversed crash, which is why it looked unfixed.

tsc never flagged it: apps/mobile/tsconfig.json extends Expo's base, not the repo's tsconfig.base.json, so noUncheckedIndexedAccess is off there and projectsForEnvironment[0] ?? null types as non-null.

Fix: accept null for the project argument and treat it like an absent project.

Verification

vp test run packages/shared/src/projectSettings.test.ts — new case fails on main with the exact production error and passes here:

# before
 FAIL  resolveProjectSettings > treats a null project like an absent one before the shell snapshot arrives
 TypeError: Cannot read properties of null (reading 'defaultModelSelection')
# after
 Tests  11 passed (11)

tsc --noEmit clean for packages/shared and apps/mobile. Not a visual change.

Claude Fable 5 via Claude Code.


Devin Review

Summary by CodeRabbit

  • Bug Fixes
    • Improved project settings resolution when project data is unavailable or null.
    • Preserved default server settings until project information becomes available.
    • Added regression coverage for handling missing project and aggregate data.

Build 56 still died on every cold open. The device's expo-error.log names
it: "TypeError: Cannot read property 'defaultModelSelection' of null" in
NewTaskFlowProvider. resolveProjectSettings reads the project aggregate's
legacy fields when projectSettingsFolded is false, and the mobile flow
passes its selected project through as-is, which is null until the shell
snapshot lands. tsc did not catch it: apps/mobile extends Expo's tsconfig
without noUncheckedIndexedAccess, so `projects[0] ?? null` types as
non-null there.

Accept null in resolveProjectSettings and treat it like an absent project.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 13, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 421ea38

Macroscope's review found this PR approvable — This is a narrowly scoped mobile crash fix that handles the already-observed null project state without changing valid project-settings resolution or product defaults. It includes focused regression coverage and has no deployment, schema, security, billing, or static-analysis implications.

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

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 3afad2ca-07aa-4d10-9ee2-92c1ff47060a

📥 Commits

Reviewing files that changed from the base of the PR and between d7c71f9 and 421ea38.

📒 Files selected for processing (2)
  • packages/shared/src/projectSettings.test.ts
  • packages/shared/src/projectSettings.ts

Included review availability: 9 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.


📝 Walkthrough

Walkthrough

resolveProjectSettings now accepts a nullable legacy project. Null project data is treated as absent. A regression test verifies that null project and aggregate inputs preserve default server settings.

Changes

Project settings resolution

Layer / File(s) Summary
Nullable project resolution
packages/shared/src/projectSettings.ts, packages/shared/src/projectSettings.test.ts
resolveProjectSettings accepts null for the legacy project argument and treats it like an absent project. The test verifies default settings for null project and aggregate inputs.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 421ea

The nullable project path is covered by regression testing, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: preventing a mobile launch crash before the shell snapshot arrives.
Description check ✅ Passed The description explains the crash, root cause, fix, verification steps, and lack of UI changes. It does not use the template headings or include the checklist, but it provides the required substantiv…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/project-settings-null-project

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

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 13.6 KiB +78 B (+0.6%) 15.1 KiB
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +1 B (+0.0%) 7.3 KiB
Codex Live turn WebSocket wire 6.5 KiB 6.6 KiB +77 B (+1.2%) 7.8 KiB
Codex Live turn WebSocket decoded 57.0 KiB 57.1 KiB +88 B (+0.2%) 66.4 KiB
Codex Live turn messages 8 10 +2 (+25.0%) 21
Claude Total thread wire 13.6 KiB 13.6 KiB −35 B (−0.3%) 15.1 KiB
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +4 B (+0.1%) 7.3 KiB
Claude Live turn WebSocket wire 6.5 KiB 6.5 KiB −39 B (−0.6%) 7.8 KiB
Claude Live turn WebSocket decoded 57.8 KiB 57.8 KiB −44 B (−0.1%) 66.4 KiB
Claude Live turn messages 9 8 −1 (−11.1%) 21

Baseline: d7c71f9 · PR result: 421ea38 · 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: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

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

@juliusmarminge
juliusmarminge merged commit 8b3ddf5 into main Sep 13, 2026
21 checks passed
@juliusmarminge
juliusmarminge deleted the fix/project-settings-null-project branch September 13, 2026 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant