feat(swift-sdk): in-app log export for beta diagnostics#4131
Merged
Conversation
Adds an Export Logs row to SwiftExampleApp's Settings tab that zips the most recent SDK log sessions (current + up to two before it, 15 MB raw cap) together with a summary.txt of build/device/network context, and hands the archive to a share sheet (AirDrop / Mail / Files). Nothing is uploaded automatically — export is user-initiated. To make this useful outside the simulator, file logging is no longer simulator-gated: devices now write the same per-session run.log files, with startup pruning to the newest 20 sessions so disk use stays bounded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
requested review from
ZocoLini,
llbartekll and
shumkov
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughSwiftDashSDK now creates and retains file-log session directories. The Swift example app selects recent sessions, packages them with diagnostic metadata into a zip archive, and provides a Diagnostics UI for sharing the archive or displaying export failures. ChangesLog session export
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant OptionsView
participant LogExporter
participant LoggingPreferences
participant FileManager
OptionsView->>LogExporter: export(network, appVersion, currentSession)
LogExporter->>LoggingPreferences: read logsRootDirectory
LogExporter->>FileManager: select and stage log sessions
LogExporter->>FileManager: write summary.txt and zip archive
LogExporter-->>OptionsView: archive URL or export error
OptionsView->>OptionsView: present share sheet or failure alert
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Collaborator
|
⛔ Blockers found — Sonnet deferred (commit fd6e7e4) |
thepastaclaw
suggested changes
Jul 15, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The user-initiated export flow is appropriately scoped, but four verified lifecycle and metadata defects make its diagnostics unreliable and leave device storage unbounded. Logging retries create false sessions, export selection can omit the active session, normal builds report a stale commit, and the new behavior has no automated coverage.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 4 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/SDKLogger.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/SDKLogger.swift:54-60: Make logging configuration idempotent across bootstrap retries
`retryBootstrap()` calls `bootstrap()` again, which invokes `LoggingPreferences.configure()` on every attempt. The Rust initializer creates seven log files and writes `build_info.txt` before `try_init()` discovers that the process-global subscriber is already installed. Each delayed retry therefore leaves a newer empty session directory, and pruning is skipped because the initializer returned false. The console fallback also cannot replace the existing subscriber, so logging continues into the original directory while the exporter sees the empty retry directories as newer sessions. Configure logging only once per process, or make the FFI initializer check subscriber availability before creating the session.
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/SDKLogger.swift:26-29: Bound retained logs by bytes, not only session count
Retaining20 directory names does not bound device storage as claimed. The Rust backend opens every log in append mode without rotation or a size check, so the active session and every retained session can be arbitrarily large. Development iOS artifacts additionally emit runtime metrics every second; release artifacts still have unbounded info-level `run.log` growth. The exporter's15 MB limit only controls what is shared, not what remains on disk. Add active-file rotation or enforce a total byte quota now that file logging is enabled on physical devices.
In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/LogExporter.swift`:
- [BLOCKING] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/LogExporter.swift:61-75: Select the authoritative current session before sorting older sessions
The exporter ignores `LoggingPreferences.currentSessionDirectory` and assumes the lexicographically newest timestamp is the active session. A clock rollback, a future-dated stale directory, or the empty directories produced by bootstrap retries can rank ahead of the real writer. Three such entries make `prefix(maxSessions)` omit the active session entirely, while a large earlier entry can trigger the byte-cap break before it is selected. The summary then labels the wrong directory as current. Pass the recorded current session into the export operation and include it first, applying timestamp sorting and the byte cap only to older sessions.
- [BLOCKING] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/LogExporter.swift:182: Generate the exported commit identifier during the build
`AppVersion.gitCommit` is the checked-in value `814df`, while this PR's head is `72412`. The app target has no shell-script build phase and the shared scheme has no pre-action invoking `generate-version.sh`. Even if invoked, that script writes `${SRCROOT}/Version.swift`, while the compiled synchronized source is `${SRCROOT}/SwiftExampleApp/Version.swift`. Normal builds therefore cannot refresh the value, and every diagnostic archive reports misleading commit metadata. Integrate commit generation into the target build and write the generated value to a compiled source or build setting.
- [SUGGESTION] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/LogExporter.swift:48-75: Add tests for log selection and logging lifecycle
No tests were added for the new selection, byte-cap, archive, pruning, or repeated-configuration behavior. Add tests with injectable temporary log roots and an explicit current session to verify archive contents, current-session inclusion, cap behavior, retention, and bootstrap retry idempotence. Use an isolated process for the global Rust subscriber lifecycle where necessary.
- configure() installs the logging pipeline at most once per process, so bootstrap retries no longer create decoy empty session dirs - startup pruning enforces a 100 MB total-bytes quota in addition to the 20-session count cap (current session always kept) - the exporter takes the authoritative currentSessionDirectory from LoggingPreferences instead of trusting timestamp order; selection is a pure, unit-tested function - summary.txt drops the stale checked-in AppVersion.gitCommit and points at the per-session build_info.txt, which records the exact compile-time platform-wallet commit Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
suggested changes
Jul 15, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
At exact head fd6e7e4, the fixes correctly make logging installation idempotent, select the authoritative current session, and export build-generated provenance. Device log storage remains unbounded during a long-running current session, and the logging lifecycle and retention fixes still lack regression coverage.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (failed),gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/SDKLogger.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/SDKLogger.swift:136-147: The current session can still grow without a byte bound
[prior id=cec1a78fbe14] The 100 MB quota runs only during startup pruning, which unconditionally retains the current directory even when it exceeds the limit. The Rust logger opens seven files with append mode and installs those writers for the process lifetime (`rs-platform-wallet-ffi/src/logging.rs:39-60,142-151`), with no rotation or live size enforcement. A long-running device session can therefore grow indefinitely until a later launch, contradicting the PR's stated requirement that device log storage remain bounded. Additionally, when a clock rollback makes the current session sort behind newer-stamped old directories, entries retained before reaching the current directory can leave the startup total above 100 MB as well. Add live file rotation or another enforceable current-session limit and ensure pruning cannot retain earlier history after the current directory pushes the total over quota.
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/SDKLogger.swift:56-77: Cover the logging lifecycle fixes with regression tests
[prior id=c0db94d40ec7] `LogExporterSelectionTests` now covers authoritative selection, ordering, count limits, and export byte limits. However, the test tree still has no coverage for repeated `configure()` calls, preservation of `currentSessionDirectory`, or startup pruning across the session-count, retained-byte, and current-session rules at `SDKLogger.swift:116-169`. These paths implement the fixes for two prior blocking regressions and should have focused lifecycle and filesystem-policy tests.
Comment on lines
+136
to
+147
| if session.lastPathComponent == currentName { | ||
| // Counted against the byte quota so a huge | ||
| // just-finished-syncing session pushes old | ||
| // history out, but never deleted itself. | ||
| keptCount += 1 | ||
| keptBytes += directoryBytes(of: session) | ||
| continue | ||
| } | ||
| let bytes = directoryBytes(of: session) | ||
| if keptCount >= maxRetainedSessions | ||
| || keptBytes + bytes > maxRetainedBytes { | ||
| try? fm.removeItem(at: session) |
Collaborator
There was a problem hiding this comment.
🔴 Blocking: The current session can still grow without a byte bound
[prior id=cec1a78fbe14] The 100 MB quota runs only during startup pruning, which unconditionally retains the current directory even when it exceeds the limit. The Rust logger opens seven files with append mode and installs those writers for the process lifetime (rs-platform-wallet-ffi/src/logging.rs:39-60,142-151), with no rotation or live size enforcement. A long-running device session can therefore grow indefinitely until a later launch, contradicting the PR's stated requirement that device log storage remain bounded. Additionally, when a clock rollback makes the current session sort behind newer-stamped old directories, entries retained before reaching the current directory can leave the startup total above 100 MB as well. Add live file rotation or another enforceable current-session limit and ensure pruning cannot retain earlier history after the current directory pushes the total over quota.
source: ['codex']
Comment on lines
+56
to
+77
| /// The tracing subscriber is process-global and first-init-wins, | ||
| /// so the install must run at most once per process. Without | ||
| /// this guard, every `configure()` call after the first (e.g. | ||
| /// a bootstrap retry) would make the Rust initializer lay out a | ||
| /// fresh session directory of empty log files before `try_init` | ||
| /// discovers the existing subscriber and bails — leaving decoy | ||
| /// "newest" sessions that logging never writes to. | ||
| @MainActor | ||
| private static var didInstallLogging = false | ||
|
|
||
| @discardableResult | ||
| @MainActor | ||
| public static func configure() -> LoggingPreset { | ||
| let preset = loadPreset() | ||
| let enableSwiftVerbose: Bool | ||
|
|
||
| // File logging is gated to the iOS Simulator | ||
| #if targetEnvironment(simulator) | ||
| if let sessionRoot = launchLogPaths(), | ||
| SDK.enableFileLogging(level: .info, sessionRoot: sessionRoot) { | ||
| } else { | ||
| SDK.enableLogging(level: .info) | ||
| if !didInstallLogging { | ||
| didInstallLogging = true | ||
| if let sessionRoot = launchLogPaths(), | ||
| SDK.enableFileLogging(level: .info, sessionRoot: sessionRoot.path) { | ||
| currentSessionDirectory = sessionRoot | ||
| pruneOldSessions(keeping: sessionRoot) |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: Cover the logging lifecycle fixes with regression tests
[prior id=c0db94d40ec7] LogExporterSelectionTests now covers authoritative selection, ordering, count limits, and export byte limits. However, the test tree still has no coverage for repeated configure() calls, preservation of currentSessionDirectory, or startup pruning across the session-count, retained-byte, and current-session rules at SDKLogger.swift:116-169. These paths implement the fixes for two prior blocking regressions and should have focused lifecycle and filesystem-policy tests.
source: ['codex']
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
2 participants
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.
Issue being fixed or feature implemented
Beta testers hitting crashes or sync issues have no way to get diagnostics off their device — the per-session Rust tracing logs under
Library/Logs/SwiftDashSDK/<timestamp>/were exactly what diagnosed the recent startup crash-loop (rust-dashcore#892/#893) and the sync-height regression (#4130), but so far the only way to retrieve them wasget_logs.shagainst a simulator. Requested by @dustinface in team chat.What was done?
LogExporter(app-side only): includes the current session plus up to two before it (after a crash, the crashed run is usually the previous session — no.ipsparsing needed), capped at 15 MB raw input; always includes the current session. Adds asummary.txtwith app version, git commit, OS, hardware, network, and per-session sizes/roles. Zipping usesNSFileCoordinator.forUploading— no third-party dependencies.LoggingPreferences(SwiftDashSDK): removed the simulator-only gate so real devices also write per-sessionrun.logfiles (otherwise there would be nothing to export in the field), exposedlogsRootDirectory/currentSessionDirectory, and added startup pruning to the newest 20 sessions so device disk use stays bounded.No FFI or Rust changes —
platform_wallet_enable_file_loggingwas already exposed; this is pure Swift app/SDK UI + file handling perpackages/swift-sdk/CLAUDE.md.How Has This Been Tested?
bash packages/swift-sdk/build_ios.sh --target sim(Rust xcframework + SwiftDashSDK + SwiftExampleApp, warnings-as-errors) — BUILD SUCCEEDED.SwiftDashSDK-logs-2026-07-15T04-25-36Z.zip(316 KB).run.log/metrics.logfiles intact) plussummary.txt. The size cap was exercised for real — the third-newest session (>10 MB) was correctly dropped, and the summary reported "Included 2 of 14 session(s) … (limit: 3 sessions / 15.7 MB raw)".Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests