Stabilize iOS simulator startup and crash mitigation flow#1836
Stabilize iOS simulator startup and crash mitigation flow#1836transphorm merged 7 commits intodevfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds a deterministic UDID-based iOS simulator launcher, crash-artifact collection script, conditional Podfile logic for arm64 simulator support, Sentry runtime-flagging to disable heavy integrations on simulators, unit tests for Sentry flags, and documentation/spec updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (runs yarn ios)
participant Script as run-ios-simulator.cjs
participant Xcrun as xcrun simctl
participant Simulator as iOS Simulator
participant Metro as react-native run-ios
participant Podfile as CocoaPods/Build
participant Sentry as Sentry SDK
Dev->>Script: invoke yarn ios (with optional IOS_SIMULATOR_RUNTIME/DEVICE)
Script->>Xcrun: xcrun simctl list --json
Xcrun-->>Script: device & runtime JSON
Script->>Script: selectDevice() filter/sort by runtime & device priority
Script->>Simulator: shutdown all booted simulators (best-effort)
Script->>Simulator: boot selected UDID
Simulator-->>Script: boot complete signal
Script->>Metro: run react-native run-ios --udid <UDID>
Metro->>Podfile: CocoaPods build (EXCLUDED_ARCHS decision)
Podfile-->>Metro: build with native arm64 or Rosetta fallback
Metro->>Sentry: App initializes Sentry with runtime flags
Sentry-->>Metro: integrations enabled/disabled based on isIosSimulator()
Metro-->>Dev: app launched
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 19414827 | Triggered | Generic Password | 78252fb | app/ios/PassportReaderCore.swift | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
specs/ios-crash-fix/SPEC.md (2)
94-94: Fix heading level hierarchy.The heading jumps from h3 (
###) to h5 (#####), skipping h4. Markdown heading levels should increment by one level at a time.📝 Proposed fix
-##### Input / Output — Chunk Validation +#### Input / Output — Chunk Validation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/ios-crash-fix/SPEC.md` at line 94, The heading "Input / Output — Chunk Validation" is at level h5 (#####) but follows a h3 section, breaking the heading hierarchy; change this heading to h4 (#### Input / Output — Chunk Validation) so headings increment by one level at a time and maintain proper Markdown structure, updating only the heading marker for the line containing "Input / Output — Chunk Validation".
102-102: Add language specifiers to fenced code blocks.Several code blocks are missing language identifiers, which affects syntax highlighting and markdown rendering. Console output blocks should use
consoleortext, and the dependency graph should usetextormermaidif it's meant to be rendered as a diagram.📝 Proposed fix
**Expected Output:** -``` +```console Simulator: iPhone 16 Pro (com.apple.CoreSimulator.SimRuntime.iOS-18-4) UDID: A1B2C3D4-E5F6-...Apply similar changes to the other code blocks at lines 114, 122, and 142.
Also applies to: 114-114, 122-122, 142-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/ios-crash-fix/SPEC.md` at line 102, Several fenced code blocks in the iOS crash spec (the Simulator/UDID console output block and the other console/dependency graph blocks) lack language identifiers; update each fenced block by adding the appropriate language specifier—use `console` or `text` for console/output snippets (e.g., the "Simulator: iPhone..."/UDID block) and use `text` or `mermaid` for the dependency graph block—so syntax highlighting and rendering work correctly; apply the same change to the remaining similar blocks referenced in the spec.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specs/ios-crash-fix/SPEC.md`:
- Around line 7-10: Add an explicit reference to specs/TEMPLATES.md in the
"Required References" section and state that this document uses the "full
implementation spec" tier from that template; update the SPEC.md Required
References block to include "- [TEMPLATES.md](../TEMPLATES.md)" and add a
one-line justification (e.g., "Tier: full implementation spec — chosen because
this is a large multi-PR feature requiring detailed planning and acceptance
criteria") so reviewers can see why the full-tier template was selected.
- Line 132: Update the inaccurate AGENTS.md line reference in SPEC.md by
replacing the text "app/AGENTS.md:107-119" with the correct range
"app/AGENTS.md:113-119"; locate the occurrence in SPEC.md (search for the exact
string "app/AGENTS.md:107-119") and change it so the spec correctly references
the Development Tips section lines 113-119 in AGENTS.md.
---
Nitpick comments:
In `@specs/ios-crash-fix/SPEC.md`:
- Line 94: The heading "Input / Output — Chunk Validation" is at level h5
(#####) but follows a h3 section, breaking the heading hierarchy; change this
heading to h4 (#### Input / Output — Chunk Validation) so headings increment by
one level at a time and maintain proper Markdown structure, updating only the
heading marker for the line containing "Input / Output — Chunk Validation".
- Line 102: Several fenced code blocks in the iOS crash spec (the Simulator/UDID
console output block and the other console/dependency graph blocks) lack
language identifiers; update each fenced block by adding the appropriate
language specifier—use `console` or `text` for console/output snippets (e.g.,
the "Simulator: iPhone..."/UDID block) and use `text` or `mermaid` for the
dependency graph block—so syntax highlighting and rendering work correctly;
apply the same change to the remaining similar blocks referenced in the spec.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7c8dea3-35e8-4973-8a14-2cee49070e49
📒 Files selected for processing (1)
specs/ios-crash-fix/SPEC.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/scripts/capture-ios-crash-artifacts.cjs`:
- Around line 133-143: The unified log capture in captureUnifiedLog calls
runCommand('log', [...]) which ultimately uses execFileSync with the default
1MiB stdout buffer and can throw ERR_CHILD_PROCESS_STDOUT_MAXBUFFER_EXCEEDED for
multi-minute logs; fix by increasing the child-process maxBuffer for this call
(e.g., to 10–50 MiB) either by updating runCommand to accept/merge exec options
and passing {maxBuffer: <size>} from captureUnifiedLog, or by adding a dedicated
execFileSync invocation with an increased maxBuffer when running the 'log'
command (keep WINDOW_MINUTES and predicate logic unchanged).
In `@app/scripts/run-ios-simulator.cjs`:
- Around line 56-65: The runtime filtering currently uses substring matching
which allows "18.1" to match "iOS-18-10"; change the filter to require an exact
runtime token match instead of includes. Use the normalized value from
normalizeRuntimeVersion(runtimeFilter) and compare against the full simulator
runtime identifier (the entries in runtimeKeys) with equality or a precise
endsWith/regex that matches the complete "iOS-${normalizedRuntime}" token (e.g.,
require the runtime string to end with `/SimRuntime.iOS-${normalizedRuntime}` or
equal the expected identifier) so only the exact requested IOS_SIMULATOR_RUNTIME
is accepted; update the filter that operates on runtimeKeys and the thrown error
remains the same.
- Around line 84-98: The current selection logic using
availableIPhones.find(...) with includes(...) can match substrings (e.g.,
"iPhone 16 Pro" matching "iPhone 16 Pro Max"); change the lookup to first try a
case-insensitive exact match against device.name (using
device.name.toLowerCase() === deviceFilter) and only if that fails fall back to
the existing substring search
(device.name.toLowerCase().includes(deviceFilter)); update the code that sets
selectedDevice (and any early returns) so availableIPhones, selectedDevice,
deviceFilter and the IOS_SIMULATOR_DEVICE-derived filter are used in that
two-step search order to ensure deterministic pinning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa9dc7e2-2157-42f8-b093-0839a1b03f97
⛔ Files ignored due to path filters (2)
app/Gemfile.lockis excluded by!**/*.lockapp/ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
app/AGENTS.mdapp/ios/Podfileapp/ios/Self.xcodeproj/project.pbxprojapp/scripts/capture-ios-crash-artifacts.cjsapp/scripts/run-ios-simulator.cjsapp/src/config/sentry.tsapp/tests/src/config/sentry.test.tsspecs/ARCHIVE.mdspecs/archive/IOS-CRASH-FIX.md
There was a problem hiding this comment.
🧹 Nitpick comments (2)
specs/archive/IOS-CRASH-FIX.md (2)
100-100: Heading level jumps from h3 to h5.Line 74 uses
### Chunk 1:(h3), but line 100 uses##### Input / Output(h5), skipping h4. This breaks document outline structure.📝 Suggested fix
-##### Input / Output — Chunk Validation +#### Input / Output — Chunk Validation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/archive/IOS-CRASH-FIX.md` at line 100, The document has a heading level jump: change the "##### Input / Output — Chunk Validation" heading (currently using "##### Input / Output") to an h4 heading ("#### Input / Output — Chunk Validation") so it follows the preceding "### Chunk 1:" section and preserves proper outline hierarchy; also scan other chunk sections for similar mismatched headings (e.g., any occurrences of "##### Input / Output" or other headings following "### Chunk X:") and adjust them to "####" where appropriate.
110-116: Add language identifiers to fenced code blocks.Several code blocks lack language specifiers, which breaks syntax highlighting and some markdown linters. The expected output blocks at lines 110, 124, and 134 appear to be shell/console output; line 154 appears to be a text diagram.
📝 Suggested fixes
-``` +```text Simulator: iPhone 16 Pro (com.apple.CoreSimulator.SimRuntime.iOS-18-4)-``` +```text iOS simulator launch failed: No available iPhone matching "iPad". Available: iPhone 16 Pro, iPhone 16, ...-``` +```text iOS simulator launch failed: No iOS runtime matching "99.0" found-``` +```text Chunk 1 (no deps) → Chunk 2 (after 1, parallel-safe but logically after)Also applies to: 124-126, 134-136, 154-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/archive/IOS-CRASH-FIX.md` around lines 110 - 116, Multiple fenced code blocks in the IOS-CRASH-FIX.md document (the blocks containing "Simulator: iPhone 16 Pro (...)", the "iOS simulator launch failed: No available iPhone ..." and "No iOS runtime matching ..." messages, and the diagram like "Chunk 1 (no deps) → Chunk 2 ...") are missing language identifiers; update each fenced block to include a language specifier such as ```text (or ```console for terminal output) for the simulator/cli outputs and ```text for the diagram so linters and syntax highlighters treat them correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@specs/archive/IOS-CRASH-FIX.md`:
- Line 100: The document has a heading level jump: change the "##### Input /
Output — Chunk Validation" heading (currently using "##### Input / Output") to
an h4 heading ("#### Input / Output — Chunk Validation") so it follows the
preceding "### Chunk 1:" section and preserves proper outline hierarchy; also
scan other chunk sections for similar mismatched headings (e.g., any occurrences
of "##### Input / Output" or other headings following "### Chunk X:") and adjust
them to "####" where appropriate.
- Around line 110-116: Multiple fenced code blocks in the IOS-CRASH-FIX.md
document (the blocks containing "Simulator: iPhone 16 Pro (...)", the "iOS
simulator launch failed: No available iPhone ..." and "No iOS runtime matching
..." messages, and the diagram like "Chunk 1 (no deps) → Chunk 2 ...") are
missing language identifiers; update each fenced block to include a language
specifier such as ```text (or ```console for terminal output) for the
simulator/cli outputs and ```text for the diagram so linters and syntax
highlighters treat them correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7abc2b94-2665-4a5e-a5b1-32ad30a2139c
📒 Files selected for processing (3)
app/scripts/capture-ios-crash-artifacts.cjsapp/scripts/run-ios-simulator.cjsspecs/archive/IOS-CRASH-FIX.md
🚧 Files skipped from review as they are similar to previous changes (1)
- app/scripts/capture-ios-crash-artifacts.cjs
This PR hardens the local iOS simulator path and reduces simulator-specific startup risk without changing real-device behavior.
It makes yarn ios deterministic by selecting a simulator by UDID, explicitly booting it, waiting for boot completion, and documenting local simulator selection controls. It also disables
Sentry mobile replay and feedback screenshots on iOS simulators only, to reduce cold-start overhead during local development. Finally, it audits binary pod simulator architecture support
during pod install and keeps the current Rosetta fallback only when required by installed dependencies; the current blocker is SwiftyTesseract/libtesseract.xcframework, which still lacks an
arm64 simulator slice.
Validation included script-level tests for simulator selection, Jest coverage for the simulator-only Sentry flags, pod install, and successful iOS simulator boots through the normal Xcode
flow.
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests