Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Missing Android exit history is incorrectly treated as proof of no prior crash, allowing premature replay.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds platform-specific prior-run eligibility to shared-core’s startup replay gate.
Changes:
- Derives eligibility from Android exit data and the iOS sentinel.
- Threads eligibility through Swift/Rust and Kotlin/JNI bridges.
- Updates shared-core pins, tests, and changelog.
File summaries
| File | Description |
|---|---|
test/platform/swift/unit_integration/mocks/MockLoggerBridgingFactory.swift |
Records replay eligibility. |
test/platform/swift/unit_integration/core/PreviousRunInfoControllerTests.swift |
Tests iOS sentinel classification. |
test/platform/swift/unit_integration/core/network/helper/NetworkTestEnvironment.swift |
Supplies unknown eligibility. |
test/platform/swift/unit_integration/core/LoggerTests.swift |
Tests iOS bridge propagation and gating. |
platform/swift/source/StartupReplayEligibility.swift |
Defines Swift bridge values. |
platform/swift/source/src/bridge.rs |
Passes iOS eligibility into shared-core. |
platform/swift/source/reports/PreviousRunInfoRepository.m |
Adds read-only sentinel loading. |
platform/swift/source/reports/PreviousRunInfoRepository.h |
Exposes sentinel loading API. |
platform/swift/source/reports/previousRunInfo/PreviousRunInfoController.swift |
Classifies prior iOS termination. |
platform/swift/source/LoggerBridgingFactoryProvider.swift |
Extends factory protocol. |
platform/swift/source/LoggerBridgingFactory.swift |
Forwards eligibility. |
platform/swift/source/LoggerBridge.swift |
Bridges eligibility to C/Rust. |
platform/swift/source/Logger.swift |
Computes the gated iOS hint. |
platform/swift/source/CaptureRustBridge.h |
Updates the C ABI. |
platform/jvm/core/src/jni.rs |
Passes Android eligibility into shared-core. |
platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/StartupReplayEligibilityTest.kt |
Verifies native values. |
platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/reports/exitinfo/PreviousRunInfoResolverTest.kt |
Tests Android classifications. |
platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/ConfigurationTest.kt |
Verifies bridge argument propagation. |
platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureLoggerNetworkTest.kt |
Updates native test setup. |
platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/StartupReplayEligibility.kt |
Defines Android bridge values. |
platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/reports/exitinfo/PreviousRunInfoResolver.kt |
Derives Android eligibility. |
platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/LoggerImpl.kt |
Sends eligibility to JNI. |
platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/IBridge.kt |
Extends the bridge contract. |
platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/CaptureJniLibrary.kt |
Updates the JNI declaration. |
MODULE.bazel.lock |
Refreshes Bazel dependency state. |
CHANGELOG.md |
Documents startup replay behavior. |
Cargo.toml |
Pins the required shared-core revision. |
Cargo.lock |
Refreshes Rust dependency resolution. |
Review details
- Files reviewed: 26/28 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Android Size Comparison Report (x86_64)
|
iOS Size Comparison Report
|
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The cross-platform ABI changes depend on still-open shared-core PR #628 and require final bridge verification.
Review details
- Files reviewed: 26/28 changed files
- Comments generated: 0 new
- Review effort level: Balanced
snowp
force-pushed
the
sp/startup-replay-crash-hint
branch
from
September 10, 2026 16:59
9cc10de to
952f472
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Current-run tracking starts too late, allowing a startup crash to leave a stale clean-exit sentinel.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 17/19 changed files
- Comments generated: 2
- Review effort level: Balanced
snowp
force-pushed
the
sp/startup-replay-crash-hint
branch
3 times, most recently
from
September 10, 2026 21:35
1d9131d to
5e0759b
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Current-run tracking still begins after core startup, leaving an untracked crash window.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 19/21 changed files
- Comments generated: 1
- Review effort level: Balanced
| Logger.issueReporterInitResult = (.initialized(.clientNotEnabled), 0) | ||
| Logger.previousRunInfoValue = .unknown | ||
| } else { | ||
| let isTrackingPreviousRun = previousRunInfoController?.startTrackingCurrentRun() == true |
snowp
force-pushed
the
sp/startup-replay-crash-hint
branch
from
September 14, 2026 15:57
5e0759b to
1afb508
Compare
Android Benchmark Results
Allocations
Timing
|
Co-Authored-By: GPT-5 <codex@openai.com>
Co-Authored-By: GPT-5 <codex@openai.com>
Co-Authored-By: GPT-5 <codex@openai.com>
Keep the controller's early read-only state through startup and defer current-run sentinel creation until crash-reporter setup. Restore the PR's required shared-core revision after rebasing onto a base that predates it. Co-Authored-By: GPT-5 <codex@openai.com>
Co-Authored-By: GPT-5 <codex@openai.com>
Co-Authored-By: GPT-5 <codex@openai.com>
Co-Authored-By: GPT-5 <codex@openai.com>
snowp
force-pushed
the
sp/startup-replay-crash-hint
branch
from
September 15, 2026 14:06
1afb508 to
e2d8145
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Failed logger construction can leave a false unclean-run sentinel that delays the next startup.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Balanced
Comment on lines
+183
to
+185
| let previousRunInfoController = CrashReporterService.isMonitoringEnabled(fromCachedConfigAt: directoryURL) | ||
| ? PreviousRunInfoController(baseDirectory: directoryURL, osVersion: clientAttributes.osVersion) | ||
| : nil |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Capture SDK now passes iOS prior-run confidence to shared-core’s startup replay gate. Before core startup, it initializes the existing previous-run controller only when cached
crash_reporting.enabledpermits monitoring, passes the controller’s hint to shared-core, and then reuses that controller unchanged for KSCrash and MetricKit’s later classification. A clean prior sentinel skips the delay; an unclean or unavailable state remains conservative. Android continues to use the conservative unknown eligibility.