Add comprehensive privacy-safe iOS app diagnostics - #9977
Merged
Merged
Conversation
Contributor
|
Too many files changed for review (134 files, 100 file limit). Bypass the limit by tagging |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds privacy-safe diagnostic contracts and records structured events across mobile application, chat, browser, terminal, settings, attachment, notification, pairing, and lifecycle flows. It also wires the shared logger through application composition and adds validation tests. ChangesDiagnostic telemetry
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (21 passed)
✨ Finishing Touches 💡 1📝 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 29
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+NotificationFeed.swift (1)
160-208: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReport one affected item for both read-state mutations.
Lines 165, 192, and 207 report
count: 0when marking an item unread. The operation still affects one item. The combined guard also reportsendpointUnavailablewhenitem.isRead == isRead, although that path is a no-op.Use
count: 1for either mutation. Emit no event, or a distinct no-op event, when the requested state already matches the item state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+NotificationFeed.swift around lines 160 - 208, Update the notification read-state mutation flow around the target guard and all notificationFeedItemMarkedRead recordAppEvent calls to skip event reporting when item.isRead already equals isRead, while preserving the no-op behavior. For actual read or unread mutations, always report count: 1, including endpoint-unavailable and error paths, instead of deriving the count from isRead.Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+Attachments.swift (1)
47-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated attachment-staging diagnostics loop.
stageSelectedPhotos(lines 47-105) andstageSelectedFiles(lines 107-154) each implement the sameattachmentPreparationStarted/Succeeded/Failedrecording pattern, including identicalCancellationErrorhandling, inside otherwise-similar staging loops.Extract the shared staging-with-diagnostics logic (start event, success event with
correlationID/count, cancellation handling, failure classification) into one helper that both callers use with a source-specific staging closure. This reduces the risk that a future diagnostic change is applied to one path but not the other.Also applies to: 107-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet`+Attachments.swift around lines 47 - 105, The attachment preparation diagnostics are duplicated between stageSelectedPhotos and stageSelectedFiles. Extract their shared start, success, cancellation, and failure-classification behavior into a helper that accepts a source-specific staging closure, then update both methods to use it while preserving each path’s existing staging and cleanup behavior.Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+DirectorySelection.swift (1)
14-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecord
.taskDirectorySearchSucceededonly for search selections
choose(path:)uses the same callback for search results and the browse screen’s “Use” action. Pass the selection source toselectDirectory(_:)and emit this event only for search selections.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet`+DirectorySelection.swift around lines 14 - 21, Update selectDirectory(_:) to accept a selection-source indicator, and have choose(path:) pass whether the path came from search or browsing. Keep the directory update behavior unchanged, but record .taskDirectorySearchSucceeded only when the source is a search selection, not from the browse screen’s “Use” action.Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swift (1)
170-195: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winOne refresh can record both
.chatSessionListLoadFailedand.chatSessionListLoadSucceeded.When
source.sessions(...)throws andstore.chatSessionListFailureMeansUnsupported(error)returnstrue, the catch records.chatSessionListLoadFailedand then setsseedOutcome = .authoritative([]). That outcome hascanInvalidateSelection == true, so line 189 also records.chatSessionListLoadSucceededwith the samediagnosticStartedAt.The log then holds two mutually exclusive terminal events for one operation. Any success-rate or latency derivation over this taxonomy becomes wrong.
Record the failure only when the error is a real failure. Treat the "unsupported" case as its own outcome.
🔧 Proposed fix that keeps one terminal event per refresh
do { seedOutcome = .authoritative(try await source.sessions(workspaceID: workspaceID)) } catch { - store.recordAppEvent( - .chatSessionListLoadFailed, - correlationID: workspaceID, - startedAt: diagnosticStartedAt, - failure: DiagnosticFailureKind.classify(error) - ) - seedOutcome = store.chatSessionListFailureMeansUnsupported(error) - ? .authoritative([]) - : .unavailable + if store.chatSessionListFailureMeansUnsupported(error) { + // The host does not offer chat sessions. This is an + // authoritative empty list, not a load failure. + seedOutcome = .authoritative([]) + } else { + store.recordAppEvent( + .chatSessionListLoadFailed, + correlationID: workspaceID, + startedAt: diagnosticStartedAt, + failure: DiagnosticFailureKind.classify(error) + ) + seedOutcome = .unavailable + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView`+AgentChat.swift around lines 170 - 195, Update the session refresh flow around source.sessions and seedOutcome so an unsupported error is represented as its own non-success outcome rather than .authoritative([]), while preserving the empty-session behavior for applying results. Ensure .chatSessionListLoadSucceeded is not recorded for unsupported failures, leaving exactly one terminal event per refresh; retain failure logging for genuine errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/cmuxPackage/Sources/cmuxFeature/MobileAuthComposition.swift`:
- Around line 176-180: Update the authentication restore reporting around
hadCachedSessionAtLaunch so intentional cache clears from clearAuthRequested or
clearStaleAuthOnLaunch are not recorded as .authRestoreFailed. Use the
coordinator’s attempted-restore state to gate .authRestoreFailed, and record a
separate skipped-restore event when launch options intentionally clear the
cached session.
In
`@Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swift`:
- Around line 547-566: Update the attachment handling flow around the picker
staging loop and final `attachments = staged` assignment to preserve pasted
attachments while replacing only previously picker-backed attachments. Track
each attachment’s source with clear ownership, calculate picker capacity from
the remaining four-item limit after existing non-picker attachments, and
restrict selection accordingly; retain the existing preparation diagnostics and
ensure the total attachment count never exceeds the limit.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/BackingUpPairedMacStore.swift`:
- Around line 1133-1142: Update the guard failure in the paired-Mac lookup
within the backup write flow to classify a missing local record as local unknown
state rather than endpoint unavailable. Change the diagnosticLog?.recordAppEvent
call for .pairedMacBackupWriteFailed to use the appropriate .unknown failure
value, while preserving the existing correlation ID and control flow.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 7005-7018: Update the error log in the setCustomization catch
block to mark both macDeviceID and the String(describing: error) interpolation
as privacy: .private, while preserving the existing message and event recording
behavior.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+NotificationDismissSync.swift:
- Line 77: Update the error interpolation in the notification dismiss sync
logging statement to use SwiftLog privacy .private instead of .public, while
keeping the count interpolation and existing log context unchanged.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+NotificationFeed.swift:
- Around line 55-59: Update notification feed telemetry to reflect refresh
outcomes: at the aggregate event near lines 55-59, derive success from the
target refresh results and emit failure when none apply; at the per-refresh
handling near lines 899-905, emit a superseded or stale outcome instead of
.notificationFeedLoadSucceeded when applied is false.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+PairedMacPersistence.swift:
- Around line 191-203: Update performSerializedPairedMacWrite and its caller so
a scope-change early return is treated as a skipped write: set accepted to false
or return whether the serialized closure executed, record a superseded failure
event for that path, and emit pairedMacStoreWriteSucceeded and
computerRoutesUpdated only when the write actually occurred.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+TaskComposer.swift:
- Around line 190-193: In both task-template persistence guards within
MobileShellComposite+TaskComposer.swift (lines 190-193 and 214-217), separate
the failure causes: record .superseded only when signed out or
capturedGeneration is stale, and record .endpointUnavailable when
taskTemplateStore is nil. Apply the same classification to both draft
persistence and submission-settings persistence.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+WorkspaceActions.swift:
- Around line 627-633: Update the authorization-failure early-return branch in
the workspace mutation function, where
disconnectForAuthorizationFailureIfNeeded(error) returns true, to record
diagnosticKinds.failure with failureKind before returning. Preserve the existing
workspaceMutationFailure return behavior and avoid duplicating the event in the
normal failure path.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+WorkspaceListRecovery.swift:
- Around line 124-135: Capture a stable snapshot of foregroundMacDeviceID before
recording workspaceListRecoveryStarted, then use that snapshot as the
correlationID for both the start event and the deferred
workspaceListRecoverySucceeded/workspaceListRecoveryFailed event. Keep the
existing recovery status, cancellation, and count handling unchanged.
In
`@Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/ComposerPendingAttachmentTests.swift`:
- Around line 56-60: Replace the fixed 100-iteration Task.yield polling with a
DiagnosticLog completion signal or deadline-bounded poll that verifies three
events are processed before snapshotting in ComposerPendingAttachmentTests.swift
(lines 56-60). Apply the same change in MobileConnectionMethodStoreTests.swift
(lines 49-53), waiting for one processed event; preserve the existing assertions
and snapshot behavior.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift`:
- Around line 340-349: Update the Task in CMUXMobileRootView around
connectPairingURL(rawURL) to capture and use the result returned by
connectPairingURLResult(_:) instead of reading store.connectionState. Classify
.appOpenURLHandled, .appOpenURLRejected, and the corresponding failure from that
operation result, matching the existing connectAttachURL(_:) behavior.
- Around line 294-306: Make the async operations explicitly owned, cancellable,
and safe against superseded results: in CMUXMobileRootView.swift:294-306, have
AuthCoordinator own and serialize the revalidation task or bind it to a
cancellable view lifecycle; in MobileIrohSettingsModel.swift:142-166, retain and
cancel relay-test tasks by relay ID and ignore completions from superseded
tests; in MobileIrohSettingsModel.swift:199-214, make mutation work caller-owned
or retain it in the model so cancellation and supersession are explicit,
eliminating unowned fire-and-forget Task instances.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/OnboardingFlowView.swift`:
- Around line 70-76: Update the stage-change diagnostic flow around
captureSceneViewed so .onboardingStageChanged is recorded only when the stage
differs from the previously tracked stage, excluding initial onAppear and
authentication-only changes; preserve scene capture behavior for all existing
triggers and update the previous-stage value after handling the transition.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swift`:
- Around line 294-296: Update TaskComposerSheet initialization to store an
authoritative typed restoration flag immediately after computing
restoredOperationID, using whether that identifier is present. In the .onAppear
event block, replace the taskTemplateStore?.composerDraft() disk re-read with
this stored flag so .draftRestored is emitted only when initialization actually
restored the draft and no extra persistence load occurs.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet`+Attachments.swift:
- Around line 27-45: Update the limit-rejection event calls in the attachment
picker presentation methods to include a distinct failure classification for the
attachment-count limit, while preserving the existing attachments.count payload.
Add or use a dedicated event kind/code for count-limit failures, and do not
replace it with the broader resource-limit event alone; apply the same
classification consistently in both presentAttachmentPhotoPicker and
presentAttachmentFileImporter.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet`+CompletedOperationRecovery.swift:
- Around line 44-67: Update the .failure(.alreadyCompleted) branch in the result
handling switch to record the mapped diagnostic failure kind for
MobileWorkspaceMutationFailure.alreadyCompleted. Use .protocolViolation or bind
the failure and reuse failure.diagnosticFailureKind instead of recording
.endpointUnavailable, while preserving the existing recovery messaging and
reconciliation behavior.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalArtifactFilesSheet.swift`:
- Around line 201-205: Update the placeholder `sessionLoader` initialization
flow in `loadInitial` and its failure branches to create
`ChatArtifactLoader.unsupported(...)` with the available `diagnosticLog`,
matching the pattern used by `WorkspaceChatPane` and `WorkspaceDetailView`.
Ensure every placeholder assigned before or instead of the resolved loader
receives the same diagnostic log, without attempting to access the environment
value during property initialization.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift`:
- Around line 406-429: Defer the cancellation check in the isPickerPresented
dismissal branch until the end of the current run-loop turn, after
pickerSelection’s onChange has settled. Re-evaluate pickerHadSelection inside
the deferred callback before recording photoPickerCancelled, while keeping
photoPickerDidDismiss and the existing selection tracking behavior unchanged.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift`:
- Around line 160-166: The durable diagnostic log uses count inconsistently
across producers. In
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift:160-166,
update recordShortcutChange and recordCustomActionChange to encode the action
through distinct event codes or a dedicated enumerated payload field instead of
count. In
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift:686-701,
stop placing attachment count and byte total in count for the same
attachmentPreparationFailed/resourceLimitReached event pair; use one documented
unit or separate failure kinds. Document count’s unit for every affected event
code in the diagnostic taxonomy.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChangesSheet.swift`:
- Around line 200-216: Update the file-diff loading flow in
WorkspaceChangesSheet so .fileDiffLoadSucceeded is recorded only after
store.fetchFileDiff completes and the response is parsed successfully, while
thrown fetch or parse errors record .fileDiffLoadFailed. Move .fileDiffExpanded
from before the fetch to the successful terminal path, preserving maxLines as
its count; record cache hits with a documented count value or omit count, using
.fileDiffCacheHit and .fileDiffLoadFailed if those event types are not yet
defined.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift`:
- Around line 989-993: Update browser creation instrumentation across
openLocalBrowserFallback and openBrowserFromToolbar so each start, success, and
failure event uses the same correlation ID for a single creation attempt. Record
the Mac-panel creation attempt and its outcome, emit a failure event when Mac
creation is rejected, and only record browserCreateSucceeded after an actual
successful creation rather than after fallback.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView`+AgentChat.swift:
- Around line 587-626: Extract the shared dictation event/failure mapping from
TerminalComposerView.recordDictationDiagnostic and
TerminalComposerView.dictationFailure, including exhaustive handling of
ComposerDictationUnavailabilityReason, into one reusable mapping. Update both
call sites to use it so all nine dictation cases and future reason cases remain
consistent and compiler-checked.
In
`@Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileDisplaySettingsTests.swift`:
- Around line 24-27: Replace the fixed Task.yield loops that wait for
DiagnosticLog processing with deterministic completion synchronization: use a
completion signal or a deadline-bounded poll of the real processedCount() >= 2
predicate. Apply the same change in
Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileDisplaySettingsTests.swift#L24-L27
and
Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift#L28-L31;
preserve the assertions after both diagnostic events are processed.
In
`@Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift`:
- Around line 38-40: Update the test method containing the report encoding
assertion to be async throws, replace the optional encoding with throwing
JSONEncoder().encode(report), and decode the resulting data directly so encoding
failures fail the test instead of producing an empty string.
In
`@Packages/iOS/CmuxMobileToast/Tests/CmuxMobileToastTests/ToastCenterTests.swift`:
- Around line 19-22: Replace the fixed 100-iteration Task.yield loop in
ToastCenterTests with deterministic synchronization for DiagnosticLog: await its
completion signal after enqueuing both events, or, if unavailable, poll
processedCount() until it reaches 2 with a deadline and fail clearly on timeout.
Preserve the assertion that both events are processed before continuing.
In
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift`:
- Around line 608-619: Update appendToWindow(_:) to compute freshMessages by
excluding knownWindowIDs before calculating artifactCount. Count only .fileEdit
and .attachment entries from freshMessages, then emit artifactDiscovered so
replayed messages cannot generate duplicate diagnostics.
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventPresentation.swift`:
- Around line 644-682: Replace the hardcoded unknown fallback in
terminalToolbarActionName, terminalZoomActionName, primaryTabName,
searchScopeName, toolbarConfigurationActionName, feedbackRouteName,
toastStyleName, and toastDismissReasonName with one shared localized value using
the stable key diagnostics.unknown.appEventValue and an English defaultValue,
matching the existing appEventName/failureName localization pattern. Add the
corresponding en and ja catalog entries to Localizable.xcstrings, preserving the
raw value in the rendered unknown(...) text.
In
`@Packages/Shared/CmuxSentryTelemetry/Tests/CmuxSentryReportingTests/TransportSentryReporterTests.swift`:
- Around line 282-285: Add an immediate `#expect`(logs.count == 2) after
retrieving logs and before any indexed access in the relevant test, then retain
the existing attribute assertions for logs[0] and logs[1].
---
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+NotificationFeed.swift:
- Around line 160-208: Update the notification read-state mutation flow around
the target guard and all notificationFeedItemMarkedRead recordAppEvent calls to
skip event reporting when item.isRead already equals isRead, while preserving
the no-op behavior. For actual read or unread mutations, always report count: 1,
including endpoint-unavailable and error paths, instead of deriving the count
from isRead.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet`+Attachments.swift:
- Around line 47-105: The attachment preparation diagnostics are duplicated
between stageSelectedPhotos and stageSelectedFiles. Extract their shared start,
success, cancellation, and failure-classification behavior into a helper that
accepts a source-specific staging closure, then update both methods to use it
while preserving each path’s existing staging and cleanup behavior.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet`+DirectorySelection.swift:
- Around line 14-21: Update selectDirectory(_:) to accept a selection-source
indicator, and have choose(path:) pass whether the path came from search or
browsing. Keep the directory update behavior unchanged, but record
.taskDirectorySearchSucceeded only when the source is a search selection, not
from the browse screen’s “Use” action.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView`+AgentChat.swift:
- Around line 170-195: Update the session refresh flow around source.sessions
and seedOutcome so an unsupported error is represented as its own non-success
outcome rather than .authoritative([]), while preserving the empty-session
behavior for applying results. Ensure .chatSessionListLoadSucceeded is not
recorded for unsupported failures, leaving exactly one terminal event per
refresh; retain failure logging for genuine errors.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8899fcbf-e3e0-4ff3-8953-1c0aa7bbf316
📒 Files selected for processing (121)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticAppEventDetail.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticCorrelation.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventCode.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventPresentation.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticTaxonomy.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/Resources/Localizable.xcstringsPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/AppLogTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationDiagnosticEvent.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/Shared/CmuxSentryTelemetry/Sources/CmuxSentryReporting/TransportSentryReporter.swiftPackages/Shared/CmuxSentryTelemetry/Tests/CmuxSentryReportingTests/TransportSentryReporterTests.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactFolderView.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactInlineViewer.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactLoader.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactViewerModel.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactViewerPageModel.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactViewerPager.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactViewerPagerModel.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScreen.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatRowActions.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatPermissionCardView.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatQuestionCardView.swiftPackages/iOS/CmuxMobileAnalytics/Sources/CmuxMobileAnalytics/AnalyticsEmitter.swiftPackages/iOS/CmuxMobileBrowser/Sources/CmuxMobileBrowser/BrowserSurfaceDiagnosticEvent.swiftPackages/iOS/CmuxMobileBrowser/Sources/CmuxMobileBrowser/MobileBrowserPane.swiftPackages/iOS/CmuxMobileBrowser/Sources/CmuxMobileBrowser/MobileBrowserView.swiftPackages/iOS/CmuxMobileBrowserStream/Sources/CmuxMobileBrowserStream/BrowserStreamEventReceiving.swiftPackages/iOS/CmuxMobileBrowserStream/Sources/CmuxMobileBrowserStream/BrowserStreamStore.swiftPackages/iOS/CmuxMobileCamera/Sources/CmuxMobileCamera/QRCodeCaptureController.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/BackingUpPairedMacStore.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatEventSource+TerminalArtifacts.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatEventSource.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobilePairingFailure.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+AgentChat.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+AppDiagnostics.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+BrowserStream.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ConnectionRecovery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+Helpers.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+HiddenMacs.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+NotificationDismissSync.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+NotificationFeed.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+PairedMacPersistence.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+SimulatorStream.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+StateSync.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskAttachments.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskComposer.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskDirectoryList.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskModels.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalOutputDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalScrollDelivery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalTheme.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TerminalViewport.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceActions.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceChanges.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceChangesContent.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceCreateRequest.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceListRecovery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileTaskDirectoryListFailure.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileTaskTemplateStore.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileWorkspaceMutationFailure.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/ComposerPendingAttachmentTests.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileConnectionMethodStore.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileConnectionMethodStoreTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceCoordinator+Artifacts.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileDiagnosticsEnvironment.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileDisplaySettings.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePairingScannerSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePushCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsAccountSection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileShellComposite+WorkspaceChangesArtifactLoader.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/NotificationFeedPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/NotificationFeedStoreView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/NotificationFeedView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/OnboardingFlowView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/QRCodeScannerView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstringsPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/SignInView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+Attachments.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+CompletedOperationRecovery.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+DirectorySelection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+DraftState.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+ModelSelection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalArtifactFilesSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChangesSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChatPane.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+Surfaces.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+TerminalArtifacts.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+DragDrop.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileDisplaySettingsTests.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationController.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationDiagnosticEvent.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceViewDelegate.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputAccessoryAction.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swiftPackages/iOS/CmuxMobileToast/Sources/CmuxMobileToast/ToastCenter.swiftPackages/iOS/CmuxMobileToast/Tests/CmuxMobileToastTests/ToastCenterTests.swiftios/cmux/AppCompositionRoot.swiftios/cmux/CmuxAppDelegate.swiftios/cmux/Resources/Localizable.xcstringsios/cmux/cmuxApp.swiftios/cmuxPackage/Sources/cmuxFeature/CMUXMobileRootScene.swiftios/cmuxPackage/Sources/cmuxFeature/MobileAnalyticsComposition.swiftios/cmuxPackage/Sources/cmuxFeature/MobileAuthComposition.swift
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift (1)
841-848: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAwait the tap callback through a completion signal.
The fixed 1,000-yield loop can finish before the drain task invokes the tap on a busy executor. The assertion can then fail although the event is retained. Capture
livethrough anAsyncStreamor continuation and await that callback before asserting.As per coding guidelines: “Tests must await real completion signals or deadline-bounded polls of real predicates rather than fixed-duration waits before assertions.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift` around lines 841 - 848, Replace the fixed 1,000-iteration Task.yield loop in the tap-delivery test with an explicit completion signal, such as an AsyncStream or continuation, fulfilled by the live tap callback. Await that signal before asserting received events, while preserving the existing event-retention and callback behavior.Source: Coding guidelines
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskComposer.swift (1)
270-276: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not report an agent launch for a plain-shell submission.
finish(.success)always records.taskAgentLaunched.TaskComposerSheetuses this submission path for plain-shell templates, where the user action is “Open Shell” rather than an agent launch. Record this event only whenspecstarts an agent. Use a separate shell event if required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+TaskComposer.swift around lines 270 - 276, The success-event flow in finish(.success) currently records .taskAgentLaunched for every submission. Gate that event on whether spec starts an agent, preserving .taskSubmitSucceeded and .taskWorkspaceCreated for plain-shell submissions; use the appropriate separate shell event if one already exists.Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swift (1)
875-905: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRecord template mutation success only after a store mutation occurs.
taskTemplateStore?.addTemplate,updateTemplate, anddeleteTemplatesare no-ops when the store is unavailable. The following event still reports creation, update, or deletion. Guard the store before mutation, then emit the success event only after that mutation. Record an unavailable-state failure when diagnostics need to capture the missing store.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swift` around lines 875 - 905, Update addTemplate, updateTemplate, and deleteTemplates to require an available taskTemplateStore before performing mutations or recording success events. Return early when the store is unavailable, and record the appropriate unavailable-state failure event if the existing diagnostics API supports it; emit taskTemplateCreated, taskTemplateUpdated, or taskTemplateDeleted only after the corresponding store mutation succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/cmux/MobileAppLifecycleDiagnostics.swift`:
- Around line 9-11: Add a concise safety comment immediately above
MobileAppLifecycleDiagnostics declaring `@unchecked` Sendable, explaining that
notificationCenter and observers are immutable and the NSObjectProtocol
collection is the sole reason compiler verification is unavailable.
- Line 22: Update the packaged iOS privacy manifest PrivacyInfo.xcprivacy to
declare the screenshot diagnostic persisted in the App Log and forwarded through
Sentry, including the applicable diagnostic and crash data categories. Align the
manifest comment with the Sentry integration while preserving the existing
consent wording and product-interaction declaration.
In
`@Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swift`:
- Around line 92-94: Update the onDisappear handler in ChatComposerView to set
isStagingAttachments = false after cancelling attachmentStaging, ensuring
lifecycle cancellation clears the staging state and does not block performSend()
when the composer reappears.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+NotificationFeed.swift:
- Around line 759-772: Update the aggregateOutcome handling in the notification
feed retry loop so .failed remains sticky when no refresh has applied: a later
.stale result must not replace an earlier failure, while .applied should still
become the final outcome. Preserve the existing retry and fetchNotificationFeed
behavior.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift`:
- Around line 71-82: Update refresh() to use a refreshToken UUID like mutate and
testCustomRelay: assign a new token when starting the task, capture it in the
task, and only clear refreshTask and refreshToken if the captured token still
matches. Add the `@ObservationIgnored` refreshToken property alongside the
existing operation tokens, and ensure cancellation leaves no stale task handle
when the current refresh exits early.
- Around line 339-343: Remove task cancellation from the deinit methods in
MobileIrohSettingsModel and MobileAuthComposition. Add MainActor-isolated
teardown methods in both owners to cancel refreshTask, mutationTask,
relayTestTasks, and the corresponding task properties, then ensure each teardown
method is called before the instance is released.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swift`:
- Line 178: Update the onDisappear cleanup in MobileIrohSettingsView so it no
longer calls model.cancelOperations(), which cancels user-initiated mutationTask
writes. Split the cleanup to cancel only the refresh and relay-test tasks, while
allowing mutateAndWait controller writes to continue after the view disappears.
In
`@Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift`:
- Around line 168-181: Update the affected test method in
MobileIrohSettingsModelTests to be async throws, then use try `#require` to verify
log.processedCount() is at least 4 after the deadline loop. After taking the
snapshot, require report.events.count == 4 before accessing report.events[1],
preserving the existing event and failure assertions.
---
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+TaskComposer.swift:
- Around line 270-276: The success-event flow in finish(.success) currently
records .taskAgentLaunched for every submission. Gate that event on whether spec
starts an agent, preserving .taskSubmitSucceeded and .taskWorkspaceCreated for
plain-shell submissions; use the appropriate separate shell event if one already
exists.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swift`:
- Around line 875-905: Update addTemplate, updateTemplate, and deleteTemplates
to require an available taskTemplateStore before performing mutations or
recording success events. Return early when the store is unavailable, and record
the appropriate unavailable-state failure event if the existing diagnostics API
supports it; emit taskTemplateCreated, taskTemplateUpdated, or
taskTemplateDeleted only after the corresponding store mutation succeeds.
In
`@Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift`:
- Around line 841-848: Replace the fixed 1,000-iteration Task.yield loop in the
tap-delivery test with an explicit completion signal, such as an AsyncStream or
continuation, fulfilled by the live tap callback. Await that signal before
asserting received events, while preserving the existing event-retention and
callback behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 30230448-23e6-4f8a-8d86-0e59eb193af5
📒 Files selected for processing (61)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticAppEventDetail.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticCorrelation.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventPresentation.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticTaxonomy.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/Resources/Localizable.xcstringsPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationDiagnosticEvent.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swiftPackages/Shared/CmuxSentryTelemetry/Tests/CmuxSentryReportingTests/TransportSentryReporterTests.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactFolderView.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerAttachment.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerPasteboard.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Composer/ChatComposerView.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScreen.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/BackingUpPairedMacStore.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatEventSource.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+AppDiagnostics.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+BrowserStream.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+NotificationDismissSync.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+NotificationFeed.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+PairedMacPersistence.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskComposer.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceActions.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceListRecovery.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/NotificationFeedFetchOutcome.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/ComposerPendingAttachmentTests.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileConnectionMethodStoreTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceCoordinator+Artifacts.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileComposerDictationDiagnostics.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePushCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/NotificationFeedStoreView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/OnboardingFlowView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerAttachmentPickerModifier.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+Attachments.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+CompletedOperationRecovery.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalArtifactFilesSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChangesSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceChatPane.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+TerminalArtifacts.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileDisplaySettingsTests.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swiftPackages/iOS/CmuxMobileToast/Sources/CmuxMobileToast/ToastCenter.swiftPackages/iOS/CmuxMobileToast/Tests/CmuxMobileToastTests/ToastCenterTests.swiftios/cmux-ios.xcodeproj/project.pbxprojios/cmux/AppCompositionRoot.swiftios/cmux/CmuxAppDelegate.swiftios/cmux/MobileAppLifecycleDiagnostics.swiftios/cmux/cmuxApp.swiftios/cmuxPackage/Sources/cmuxFeature/MobileAuthComposition.swift
💤 Files with no reviewable changes (3)
- Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactFolderView.swift
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+TerminalArtifacts.swift
- ios/cmux/cmuxApp.swift
Comment on lines
+9
to
+11
| final class MobileAppLifecycleDiagnostics: @unchecked Sendable { | ||
| private let notificationCenter: NotificationCenter | ||
| private let observers: [NSObjectProtocol] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Document the @unchecked Sendable rationale.
Both stored properties are immutable, and [NSObjectProtocol] is the only reason the compiler cannot prove Sendable. State that in the code so the escape hatch is auditable.
♻️ Proposed change
+/// `@unchecked Sendable`: every stored property is immutable after `init`, and
+/// the observer tokens are only read in `deinit`. The opt-out exists solely
+/// because `[NSObjectProtocol]` carries no `Sendable` conformance.
final class MobileAppLifecycleDiagnostics: `@unchecked` Sendable {
private let notificationCenter: NotificationCenter
private let observers: [NSObjectProtocol]As per coding guidelines, "Do not mark shared mutable reference types as Sendable unless they use ... @unchecked Sendable with a clear safety explanation."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final class MobileAppLifecycleDiagnostics: @unchecked Sendable { | |
| private let notificationCenter: NotificationCenter | |
| private let observers: [NSObjectProtocol] | |
| /// `@unchecked Sendable`: every stored property is immutable after `init`, and | |
| /// the observer tokens are only read in `deinit`. The opt-out exists solely | |
| /// because `[NSObjectProtocol]` carries no `Sendable` conformance. | |
| final class MobileAppLifecycleDiagnostics: `@unchecked` Sendable { | |
| private let notificationCenter: NotificationCenter | |
| private let observers: [NSObjectProtocol] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/cmux/MobileAppLifecycleDiagnostics.swift` around lines 9 - 11, Add a
concise safety comment immediately above MobileAppLifecycleDiagnostics declaring
`@unchecked` Sendable, explaining that notificationCenter and observers are
immutable and the NSObjectProtocol collection is the sole reason compiler
verification is unavailable.
Source: Coding guidelines
| (UIApplication.didReceiveMemoryWarningNotification, .appMemoryWarningReceived), | ||
| (UIApplication.protectedDataWillBecomeUnavailableNotification, .appProtectedDataUnavailable), | ||
| (UIApplication.protectedDataDidBecomeAvailableNotification, .appProtectedDataAvailable), | ||
| (UIApplication.userDidTakeScreenshotNotification, .appScreenshotCaptured), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the privacy manifest and consent copy for behavioral telemetry coverage.
fd -t f 'PrivacyInfo.xcprivacy' --exec cat {}
rg -nP --type=swift -C3 'appScreenshotCaptured'
rg -nPi 'telemetry|analytics' --iglob '*.xcstrings' -lRepository: manaflow-ai/cmux
Length of output: 5911
🏁 Script executed:
#!/bin/bash
set -u
echo '--- target and related files ---'
fd -t f -i 'MobileAppLifecycleDiagnostics.swift|PrivacyInfo.xcprivacy|.*xcstrings|.*Consent.*|.*Telemetry.*|.*Analytics.*' .
echo '--- diagnostics source ---'
if [ -f ios/cmux/MobileAppLifecycleDiagnostics.swift ]; then
wc -l ios/cmux/MobileAppLifecycleDiagnostics.swift
cat -n ios/cmux/MobileAppLifecycleDiagnostics.swift
fi
echo '--- all event references ---'
rg -n -S -C4 'appScreenshotCaptured|userDidTakeScreenshotNotification|ScreenshotCaptured|screenshot' --glob '!**/DerivedData/**' --glob '!**/.git/**' .
echo '--- telemetry and consent references ---'
rg -n -i -C3 'telemetry|analytics|consent|opt.?out|sentry|breadcrumb|diagnostic' ios .github --glob '*.{swift,xcstrings,plist,json,md,yml,yaml}' 2>/dev/null | head -1200
echo '--- tracked privacy manifests ---'
git ls-files '*PrivacyInfo.xcprivacy' | while IFS= read -r f; do
echo "### $f"
cat -n "$f"
doneRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -u
echo '--- tracked target ---'
git ls-files -- 'ios/cmux/MobileAppLifecycleDiagnostics.swift' '*PrivacyInfo.xcprivacy' '*.xcstrings'
echo '--- target source ---'
cat -n ios/cmux/MobileAppLifecycleDiagnostics.swift
echo '--- exact event references ---'
rg -n -F -C6 'appScreenshotCaptured' --glob '!**/.git/**' .
rg -n -F -C6 'userDidTakeScreenshotNotification' --glob '!**/.git/**' .
echo '--- telemetry/consent identifiers in iOS app ---'
rg -n -i -C4 'telemetry|analytics|consent|opt.?out|sentry|breadcrumb|durable|diagnostic' ios/cmux --glob '*.swift' --glob '*.xcstrings' --glob '*.plist' | head -800
echo '--- privacy manifests with paths and relevant keys ---'
git ls-files '*PrivacyInfo.xcprivacy' | while IFS= read -r f; do
echo "### $f"
rg -n -C3 'NSPrivacyCollectedDataType|NSPrivacyTracking|ProductInteraction|Analytics|Diagnostic|Crash|UserID|DeviceID|EmailAddress' "$f"
doneRepository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
echo '--- exact implementation files ---'
rg -l -S 'class DiagnosticLog|struct DiagnosticLog|recordAppEvent|setEventTap|class TransportSentryReporter|struct TransportSentryReporter|isTelemetryEnabled|UserDefaultsAnalyticsConsentProvider|telemetryAnalyticsOnlyFooter|telemetryFooter' ios Packages/Shared Packages/iOS --glob '*.{swift,xcstrings,plist}' | sort -u
echo '--- DiagnosticLog and event-tap implementation ---'
rg -n -S -C12 'recordAppEvent|setEventTap|eventTap|func ingest|class DiagnosticLog|actor DiagnosticLog|struct DiagnosticLog' ios Packages/Shared Packages/iOS --glob '*.swift' | head -1000
echo '--- Sentry reporter and consent implementation ---'
rg -n -S -C14 'TransportSentryReporter|Sentry.*Reporter|isTelemetryEnabled|UserDefaultsAnalyticsConsentProvider|RevocationWatcher|SentrySDK.isEnabled|startIfEnabled' ios Packages/Shared Packages/iOS --glob '*.swift' | head -1400
echo '--- telemetry UI and exact localized copy ---'
rg -n -S -C8 'telemetryAnalyticsOnlyFooter|telemetryFooter|mobile.settings.telemetry|Share Analytics|product analytics|crash reports|App Log records' ios Packages/iOS Packages/Shared --glob '*.{swift,xcstrings}' | head -1000
echo '--- both iOS privacy manifests ---'
for f in ios/cmux/PrivacyInfo.xcprivacy ios/cmux/Resources/PrivacyInfo.xcprivacy; do
echo "### $f"
cat -n "$f"
doneRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -u
echo '--- reporter implementation ---'
f=$(rg -l -S 'public final class TransportSentryReporter' Packages/Shared --glob '*.swift' | head -1)
echo "### $f"
cat -n "$f" | sed -n '1,260p'
echo '--- DiagnosticLog implementation locations ---'
rg -l -S 'recordAppEvent|setEventTap' Packages/Shared ios --glob '*.swift' | grep -v '/Tests/' | sort -u
echo '--- privacy manifest summaries ---'
python3 - <<'PY'
import plistlib
from pathlib import Path
for p in [Path("ios/cmux/PrivacyInfo.xcprivacy"), Path("ios/cmux/Resources/PrivacyInfo.xcprivacy")]:
print(f"### {p}")
with p.open("rb") as fh:
d = plistlib.load(fh)
print("tracking:", d.get("NSPrivacyTracking"))
for item in d.get("NSPrivacyCollectedDataTypes", []):
print({
"type": item.get("NSPrivacyCollectedDataType"),
"linked": item.get("NSPrivacyCollectedDataTypeLinked"),
"tracking": item.get("NSPrivacyCollectedDataTypeTracking"),
"purposes": item.get("NSPrivacyCollectedDataTypePurposes"),
})
PY
echo '--- telemetry strings and usage ---'
rg -n -S -C12 'mobile.settings.telemetry|Share Analytics|product analytics|crash reports|App Log records|diagnostic' ios/cmux/Resources/Localizable.xcstrings ios/cmux --glob '*.swift' | grep -E 'mobile.settings.telemetry|Share Analytics|product analytics|crash reports|App Log records|diagnostic|telemetry' | head -500Repository: manaflow-ai/cmux
Length of output: 24095
🏁 Script executed:
#!/bin/bash
set -u
echo '--- manifest references and target membership ---'
rg -n -C6 'PrivacyInfo\.xcprivacy|PrivacyInfo' ios --glob '*.{pbxproj,pbxproj,xcconfig,swift,plist,json,yml,yaml}' | head -500
echo '--- telemetry settings usage ---'
rg -n -S -C12 'telemetryAnalyticsOnly|telemetryFooter|mobile.settings.telemetry|Share Analytics and Crash Reports|AnalyticsConsentProviding|isTelemetryEnabled' ios Packages/iOS Packages/Shared --glob '*.swift' | head -1000
echo '--- DiagnosticLog persistence and event classification ---'
f=Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swift
cat -n "$f" | sed -n '1,280p'
f2=Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticTaxonomy.swift
rg -n -C10 'appScreenshotCaptured|isAppFeatureDiagnosticEvent|DiagnosticAppEventKind' "$f2"Repository: manaflow-ai/cmux
Length of output: 50373
Declare the screenshot diagnostic in the packaged iOS privacy manifest.
ios/cmux/PrivacyInfo.xcprivacy declares only NSPrivacyCollectedDataTypeProductInteraction, but appScreenshotCaptured is also persisted in the App Log and forwarded to Sentry as a breadcrumb and possible structured log when consent is enabled. Add the applicable diagnostic and crash data declarations. Keep the manifest comment aligned with the Sentry integration. The existing consent copy covers product analytics and crash reports.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/cmux/MobileAppLifecycleDiagnostics.swift` at line 22, Update the packaged
iOS privacy manifest PrivacyInfo.xcprivacy to declare the screenshot diagnostic
persisted in the App Log and forwarded through Sentry, including the applicable
diagnostic and crash data categories. Align the manifest comment with the Sentry
integration while preserving the existing consent wording and
product-interaction declaration.
Comment on lines
+759
to
+772
| var aggregateOutcome = NotificationFeedFetchOutcome.failed | ||
| repeat { | ||
| self.notificationFeedRefreshPendingMacIDs.remove(macDeviceID) | ||
| let requiredRevision = | ||
| self.notificationFeedKnownRevisionsByMac[macDeviceID] ?? -1 | ||
| _ = await self.fetchNotificationFeed( | ||
| let outcome = await self.fetchNotificationFeed( | ||
| macDeviceID: macDeviceID, | ||
| client: client, | ||
| displayName: displayName, | ||
| requiredRevision: requiredRevision | ||
| ) | ||
| if outcome == .applied || aggregateOutcome != .applied { | ||
| aggregateOutcome = outcome | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Preserve a failed refresh outcome.
If an immediate retry first returns .failed and later returns .stale, Line 771 replaces the failure. Lines 63-69 then report .superseded instead of an endpoint failure when no refresh applied. Keep .failed sticky until an .applied outcome occurs.
Proposed fix
- var aggregateOutcome = NotificationFeedFetchOutcome.failed
+ var aggregateOutcome: NotificationFeedFetchOutcome?
repeat {
...
- if outcome == .applied || aggregateOutcome != .applied {
+ if aggregateOutcome != .applied,
+ (outcome == .failed || aggregateOutcome != .failed) {
aggregateOutcome = outcome
}
...
- return aggregateOutcome
+ return aggregateOutcome ?? .failed📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var aggregateOutcome = NotificationFeedFetchOutcome.failed | |
| repeat { | |
| self.notificationFeedRefreshPendingMacIDs.remove(macDeviceID) | |
| let requiredRevision = | |
| self.notificationFeedKnownRevisionsByMac[macDeviceID] ?? -1 | |
| _ = await self.fetchNotificationFeed( | |
| let outcome = await self.fetchNotificationFeed( | |
| macDeviceID: macDeviceID, | |
| client: client, | |
| displayName: displayName, | |
| requiredRevision: requiredRevision | |
| ) | |
| if outcome == .applied || aggregateOutcome != .applied { | |
| aggregateOutcome = outcome | |
| } | |
| var aggregateOutcome: NotificationFeedFetchOutcome? | |
| repeat { | |
| self.notificationFeedRefreshPendingMacIDs.remove(macDeviceID) | |
| let requiredRevision = | |
| self.notificationFeedKnownRevisionsByMac[macDeviceID] ?? -1 | |
| let outcome = await self.fetchNotificationFeed( | |
| macDeviceID: macDeviceID, | |
| client: client, | |
| displayName: displayName, | |
| requiredRevision: requiredRevision | |
| ) | |
| if aggregateOutcome != .applied, | |
| (outcome == .failed || aggregateOutcome != .failed) { | |
| aggregateOutcome = outcome | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+NotificationFeed.swift
around lines 759 - 772, Update the aggregateOutcome handling in the notification
feed retry loop so .failed remains sticky when no refresh has applied: a later
.stale result must not replace an earlier failure, while .applied should still
become the final outcome. Preserve the existing retry and fetchNotificationFeed
behavior.
Comment on lines
71
to
82
| func refresh() { | ||
| Task { | ||
| await controller.refreshIrohSettings() | ||
| snapshot = await controller.irohSettingsSnapshot() | ||
| await reloadDiagnostics() | ||
| refreshTask?.cancel() | ||
| refreshTask = Task { @MainActor [weak self] in | ||
| guard let self else { return } | ||
| await self.controller.refreshIrohSettings() | ||
| guard !Task.isCancelled else { return } | ||
| self.snapshot = await self.controller.irohSettingsSnapshot() | ||
| await self.reloadDiagnostics() | ||
| guard !Task.isCancelled else { return } | ||
| self.refreshTask = nil | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Track refreshTask with a token like the other operations.
refresh() replaces refreshTask without a supersession token. Two failure modes follow.
- If a prior refresh finishes its last
awaitbefore the cancel takes effect, it reaches Line 80 and setsrefreshTask = nil, which discards the handle of the newer task.cancelOperations()anddeinitcan then no longer cancel that task. - If the task is cancelled, Line 79 returns early and leaves
refreshTaskpointing at a finished, cancelled task until the nextrefresh().
Use the same token pattern as mutate and testCustomRelay.
🔧 Proposed fix
func refresh() {
refreshTask?.cancel()
+ let token = UUID()
+ refreshToken = token
refreshTask = Task { `@MainActor` [weak self] in
guard let self else { return }
await self.controller.refreshIrohSettings()
- guard !Task.isCancelled else { return }
+ guard !Task.isCancelled, self.refreshToken == token else { return }
self.snapshot = await self.controller.irohSettingsSnapshot()
await self.reloadDiagnostics()
- guard !Task.isCancelled else { return }
- self.refreshTask = nil
+ guard self.refreshToken == token else { return }
+ self.refreshTask = nil
+ self.refreshToken = nil
}
}Add the token storage next to the existing ones:
`@ObservationIgnored` private var refreshToken: UUID?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift`
around lines 71 - 82, Update refresh() to use a refreshToken UUID like mutate
and testCustomRelay: assign a new token when starting the task, capture it in
the task, and only clear refreshTask and refreshToken if the captured token
still matches. Add the `@ObservationIgnored` refreshToken property alongside the
existing operation tokens, and ensure cancellation leaves no stale task handle
when the current refresh exits early.
Comment on lines
+339
to
+343
| deinit { | ||
| refreshTask?.cancel() | ||
| mutationTask?.cancel() | ||
| relayTestTasks.values.forEach { $0.cancel() } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Determine Swift language mode / default isolation for the affected packages and targets.
fd -t f 'Package.swift' Packages/iOS/CmuxMobileShellUI ios/cmuxPackage --exec cat -n {}
rg -nP 'SWIFT_(VERSION|DEFAULT_ACTOR_ISOLATION|STRICT_CONCURRENCY|UPCOMING_FEATURE)' ios/cmux-ios.xcodeproj/project.pbxproj ios/Config 2>/dev/null
# Find existing precedents of deinit inside MainActor-isolated types.
ast-grep run --pattern $'`@MainActor`
$KIND $NAME {
$$$
deinit {
$$$
}
$$$
}' --lang swift Packages/iOS iosRepository: manaflow-ai/cmux
Length of output: 11108
🌐 Web query:
Swift 6 nonisolated deinit access to MainActor isolated stored properties rules
💡 Result:
In Swift 6, attempting to access MainActor-isolated stored properties from a standard, non-isolated deinitializer is disallowed and will trigger a compile-time error [1][2]. This restriction exists because a standard deinitializer is not guaranteed to run on the MainActor executor; it is executed whenever the object's final reference is released, which can occur on any thread [1][2]. To safely access MainActor-isolated properties or perform MainActor-isolated cleanup during deinitialization, Swift provides the isolated deinit feature [3][4]. Key Rules and Mechanisms: 1. Isolated Deinit: By declaring a deinitializer as isolated deinit, you instruct the Swift runtime to schedule the object's deallocation and the execution of the deinit body on the actor's executor (in this case, the MainActor) [3][5]. This allows safe, synchronous access to the class's isolated stored properties [3][4]. 2. Default Isolation: Implicitly synthesized deinitializers are nonisolated by default [3][5][6]. If you need MainActor isolation, you must explicitly declare isolated deinit [3][5]. 3. Inheritance Rules: If a base class has an isolated deinit, all derived classes must also have an isolated deinit with the same isolation to maintain safety [3][5]. 4. Compatibility: This feature is intended for classes and actors that require cleanup of non-sendable, isolated state [3][5]. For types that do not require custom cleanup logic, the default nonisolated deinitializer remains appropriate and safe for releasing references [3][5]. In scenarios where isolated deinit is not available or appropriate, developers are traditionally advised to perform cleanup in an explicit lifecycle method (e.g., stop() or invalidate()) called before the object is deallocated, or to ensure that properties are compatible with non-isolated access, such as using nonisolated(unsafe) if thread safety can be externally guaranteed [7].
Citations:
- 1: https://forums.swift.org/t/cannot-access-property-with-a-non-sendable-type-from-non-isolated-deinit/57877
- 2: https://forums.swift.org/t/mainactor-isolated-property-can-not-be-mutated-from-deinit/72371
- 3: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0371-isolated-synchronous-deinit.md
- 4: https://onmyway133.com/posts/how-to-use-isolated-and-nonisolated-in-swift-6-concurrency/
- 5: https://github.com/apple/swift-evolution/blob/main/proposals/0371-isolated-synchronous-deinit.md
- 6: https://forums.swift.org/t/default-actor-isolation-and-deinit/84511
- 7: https://stackoverflow.com/questions/78774528/how-to-deallocate-resource-in-main-actor-isolated-object-in-deinit
🏁 Script executed:
#!/bin/bash
set -u
for f in \
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift \
ios/cmuxPackage/Sources/cmuxFeature/MobileAuthComposition.swift
do
echo "=== $f ==="
rg -n -C 12 '(`@MainActor`|class MobileIrohSettingsModel|class MobileAuthTaskOwner|struct MobileAuthTaskOwner|deinit|refreshTask|mutationTask|relayTestTasks|restoreTask|revalidationTask)' "$f"
done
echo "=== package language settings ==="
rg -n -C 3 'swiftLanguageMode|SWIFT_VERSION|SWIFT_DEFAULT_ACTOR_ISOLATION|SWIFT_STRICT_CONCURRENCY|SWIFT_UPCOMING_FEATURE' \
Packages/iOS/CmuxMobileShellUI/Package.swift \
ios/cmuxPackage/Package.swift \
ios/cmux-ios.xcodeproj/project.pbxproj 2>/dev/null || true
echo "=== related task-owner declarations ==="
rg -n -C 8 'class (MobileIrohSettingsModel|MobileAuthTaskOwner)|`@MainActor`.*(MobileIrohSettingsModel|MobileAuthTaskOwner)' \
Packages/iOS iosRepository: manaflow-ai/cmux
Length of output: 21534
🌐 Web query:
Swift Evolution SE-0371 isolated synchronous deinit Swift 6 availability Task cancel MainActor
💡 Result:
Swift Evolution proposal SE-0371, "Isolated synchronous deinit," was implemented and made available in Swift 6.2 [1][2]. It allows deinitializers for actors and global-actor isolated classes to be explicitly marked as isolated deinit [1][2]. Key characteristics and behaviors of isolated deinit include: * Purpose and Functionality: It enables deinit to access non-sendable isolated state by ensuring the deinit body executes on the actor's executor or the relevant global actor's executor [1]. This lifts restrictions previously imposed on deinit when dealing with actor-isolated state [1]. * Syntax: You declare it using the isolated keyword, for example: isolated deinit { ... } [1][3]. * Scheduling: If the object is already on the correct executor, the deinit runs synchronously [1]. If switching is required, the runtime schedules a task-less job on the target executor with the same priority as the thread that released the last reference to the object [1]. * Task Locals: To ensure consistency and avoid reliance on implementation details, task locals are cleared when entering an isolated deinit [4][5]. They do not retain the values from the scope where the object was released [4][6]. * Implicit Deinitializers: Implicitly synthesized deinitializers are nonisolated by default [3]. If you write an explicit deinit, you must mark it with isolated or the appropriate global actor attribute if you wish to maintain isolation [3]. Regarding your query on Swift 6 compatibility: This feature is not part of the initial Swift 6.0 release; it was finalized for Swift 6.2 [1][7]. Because it requires runtime support for these specific execution semantics, it may have minimum OS version requirements (e.g., iOS 18.4+), and usage in earlier versions or with older OS deployment targets may not be supported or may be restricted to experimental flags in newer compilers [7].
Citations:
- 1: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0371-isolated-synchronous-deinit.md
- 2: https://github.com/apple/swift-evolution/blob/main/proposals/0371-isolated-synchronous-deinit.md
- 3: https://forums.swift.org/t/default-actor-isolation-and-deinit/84511
- 4: https://forums.swift.org/t/accepted-with-modifications-se-0371-isolated-synchronous-deinit/74042
- 5: Make isolated deinit clear task locals & remove experimental flag swiftlang/swift#76538
- 6: https://forums.swift.org/t/second-review-se-0371-isolated-synchronous-deinit/73406
- 7: https://forums.swift.org/t/isolated-deinit-not-in-swift-6-1/78055
Move task cancellation out of both deinit methods. The affected SwiftPM targets use Swift 6, and ordinary deinit cannot access these @MainActor-isolated task properties. Add MainActor teardown methods and call them before release in both owners.
📍 Affects 2 files
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift#L339-L343(this comment)ios/cmuxPackage/Sources/cmuxFeature/MobileAuthComposition.swift#L398-L401
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift`
around lines 339 - 343, Remove task cancellation from the deinit methods in
MobileIrohSettingsModel and MobileAuthComposition. Add MainActor-isolated
teardown methods in both owners to cancel refreshTask, mutationTask,
relayTestTasks, and the corresponding task properties, then ensure each teardown
method is called before the instance is released.
| .navigationTitle(L10n.string("mobile.iroh.title", defaultValue: "Iroh and Relays")) | ||
| .navigationBarTitleDisplayMode(.inline) | ||
| .task { await model.observe() } | ||
| .onDisappear { model.cancelOperations() } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the Iroh settings controller mutations are cancellation-aware mid-write.
ast-grep outline Packages/iOS --items all --match 'CmxIrohSettingsControlling'
rg -nP --type=swift -C5 'func (setPathPreference|setPreference|upsertCustomRelay|upsertCustomPrivatePath|removeCustomRelay)\s*\(' -g '!**/Tests/**'Repository: manaflow-ai/cmux
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -eu
target='Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swift'
printf '%s\n' '--- target outline ---'
ast-grep outline "$target" --items all
printf '%s\n' '--- target size ---'
wc -l "$target"
printf '%s\n' '--- relevant symbols ---'
rg -n -C8 --type=swift 'cancelOperations|mutationTask|mutateAndWait|onDisappear|Relay Only|custom relay|setPathPreference|upsertCustom|removeCustom' Packages/iOS
printf '%s\n' '--- matching model/controller files ---'
rg -l --type=swift 'cancelOperations|mutationTask|mutateAndWait|IrohSettings' Packages/iOSRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -eu
target='Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swift'
printf '%s\n' '--- target lines 1-230 ---'
sed -n '1,230p' "$target"
printf '%s\n' '--- target lines 430-664 ---'
sed -n '430,664p' "$target"
printf '%s\n' '--- exact symbol locations ---'
rg -n --type=swift 'MobileIrohSettings(Model|Controller)|cancelOperations|mutationTask|mutateAndWait|setPathPreference|upsertCustomRelay|upsertCustomPrivatePath|removeCustomPrivatePath|startRelayTest|refresh' Packages/iOS/CmuxMobileShellUIRepository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
model='Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift'
printf '%s\n' '--- model outline ---'
ast-grep outline "$model" --items all
printf '%s\n' '--- model implementation ---'
sed -n '1,380p' "$model"
printf '%s\n' '--- controller protocol and implementations ---'
rg -n -C6 --type=swift 'CmxIrohSettingsControlling|refreshIrohSettings|setIrohPathPreference|upsertIrohCustomRelay|removeIrohCustomRelay|upsertIrohCustomPrivatePath|removeIrohCustomPrivatePath' PackagesRepository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
model = Path("Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift").read_text()
view = Path("Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swift").read_text()
checks = {
"view cancels all operations": r"\.onDisappear\s*\{\s*model\.cancelOperations\(\)\s*\}",
"cancelOperations cancels mutationTask": r"func cancelOperations\(\).*?mutationTask\?\.cancel\(\)",
"mutateAndWait checks before operation": r"func mutateAndWait\(.*?try Task\.checkCancellation\(\)\s*try await operation\(\)",
"mutateAndWait checks after operation": r"try await operation\(\)\s*try Task\.checkCancellation\(\)",
"path preference uses mutate": r"func setPathPreference\(.*?mutate\(",
"custom relay save uses mutateAndWait": r"func upsertCustomRelay\(.*?await mutateAndWait\(",
"private path save uses mutateAndWait": r"func upsertCustomPrivatePath\(.*?await mutateAndWait\(",
}
for label, pattern in checks.items():
source = view if label == "view cancels all operations" else model
found = re.search(pattern, source, re.S) is not None
print(f"{label}: {'yes' if found else 'no'}")
print("controller declarations:")
for path in Path("Packages").rglob("*.swift"):
text = path.read_text(errors="replace")
if "protocol CmxIrohSettingsControlling" in text:
print(path)
start = text.index("protocol CmxIrohSettingsControlling")
print(text[start:start + 1800])
PY
printf '%s\n' '--- production protocol references ---'
rg -n --glob '*.swift' --glob '!**/Tests/**' 'CmxIrohSettingsControlling|func (setIrohPathPreference|upsertIrohCustomRelay|upsertIrohCustomPrivatePath)' PackagesRepository: manaflow-ai/cmux
Length of output: 5596
Keep user-initiated settings writes alive after the view disappears.
cancelOperations() cancels mutationTask. mutateAndWait can then stop before the controller write or record .cancelled after it completes. Split cleanup so disappearance cancels only refresh and relay-test tasks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsView.swift`
at line 178, Update the onDisappear cleanup in MobileIrohSettingsView so it no
longer calls model.cancelOperations(), which cancels user-initiated mutationTask
writes. Split the cleanup to cancel only the refresh and relay-test tasks, while
allowing mutateAndWait controller writes to continue after the view disappears.
Comment on lines
+168
to
+181
| let clock = ContinuousClock() | ||
| let deadline = clock.now.advanced(by: .seconds(1)) | ||
| while await log.processedCount() < 4, clock.now < deadline { | ||
| await Task.yield() | ||
| } | ||
|
|
||
| let report = await log.snapshot() | ||
| #expect(report.events.map(\.a) == [ | ||
| DiagnosticAppEventKind.irohCustomRelayTestStarted.rawValue, | ||
| DiagnosticAppEventKind.irohCustomRelayTestFailed.rawValue, | ||
| DiagnosticAppEventKind.irohCustomRelayTestStarted.rawValue, | ||
| DiagnosticAppEventKind.irohCustomRelayTestSucceeded.rawValue, | ||
| ]) | ||
| #expect(report.events[1].b == DiagnosticFailureKind.superseded.rawValue) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift"
printf '%s\n' '--- target test ---'
sed -n '130,195p' "$file"
printf '%s\n' '--- nearby event-log declarations and uses ---'
rg -n -C 4 'processedCount|snapshot|struct .*Log|actor .*Log|events' "$file"Repository: manaflow-ai/cmux
Length of output: 15226
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- DiagnosticLog definitions ---'
rg -n -C 12 'actor DiagnosticLog|final class DiagnosticLog|struct DiagnosticLog|processedCount\(\)|func snapshot\(\)' \
--glob '*.swift' .
printf '%s\n' '--- test wait helper and package test configuration ---'
sed -n '260,320p' Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift
rg -n -C 5 'SwiftTesting|Testing|swift-testing|`#require`|`#expect`' \
Packages/iOS/CmuxMobileShellUI/Package.swift Packages/iOS/CmuxMobileShellUI/Tests 2>/dev/null || trueRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- files defining or using processedCount ---'
rg -l --glob '*.swift' 'processedCount' . | head -50
printf '%s\n' '--- exact DiagnosticLog references ---'
rg -n --glob '*.swift' '\bDiagnosticLog\b' . | head -100
printf '%s\n' '--- target imports and wait helper ---'
sed -n '1,12p' Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift
sed -n '262,275p' Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swiftRepository: manaflow-ai/cmux
Length of output: 15453
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- DiagnosticLog implementation ---'
sed -n '1,260p' Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swift
printf '%s\n' '--- DiagnosticLog processing tests ---'
sed -n '1,75p' Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift
sed -n '300,350p' Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swiftRepository: manaflow-ai/cmux
Length of output: 16728
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relay test implementation ---'
rg -n -C 16 'testCustomRelay|irohCustomRelayTestStarted|irohCustomRelayTestFailed|irohCustomRelayTestSucceeded' \
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift
printf '%s\n' '--- DiagnosticLog barrier and snapshot declarations ---'
rg -n -C 10 'func processedCount|func snapshot|func append\(' \
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swiftRepository: manaflow-ai/cmux
Length of output: 8522
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
test = Path("Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift").read_text()
log = Path("Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swift").read_text()
model = Path("Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift").read_text()
# Read-only structural checks for the relevant behavior.
assert "while await log.processedCount() < 4, clock.now < deadline" in test
assert "report.events[1]" in test
assert "snapshot(generatedAt: Date = Date())" in log
assert "Events still pending in the non-blocking stream are not forced to drain" in log
assert "totalProcessed += 1" in log
# The timeout path is valid: the loop may exit with any processed count below 4.
states = []
for processed in range(4):
events = list(range(processed)) # no more than processed events can be retained
try:
events[1]
indexed_access = "valid"
except IndexError:
indexed_access = "trap"
states.append((processed, len(events), indexed_access))
print("timeout states:", states)
assert any(state[2] == "trap" for state in states)
assert all((count >= 2) == (access == "valid")
for _, count, access in states)
# The intended path records two starts plus superseded failure and success.
assert model.count(".irohCustomRelayTestStarted") >= 1
assert model.count(".irohCustomRelayTestFailed") >= 1
assert ".irohCustomRelayTestSucceeded" in model
print("result: the deadline can leave an insufficient event array, and report.events[1] can trap")
PYRepository: manaflow-ai/cmux
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
test = Path("Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift").read_text()
log = Path("Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swift").read_text()
model = Path("Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swift").read_text()
checks = {
"deadline loop": "while await log.processedCount() < 4, clock.now < deadline" in test,
"indexed access": "report.events[1]" in test,
"non-forcing snapshot": "Events still pending in the non-blocking stream are not forced to" in log,
"processed count declaration": "public func processedCount() async -> Int" in log,
"append increments count": "totalProcessed" in log and "processedCount()" in log,
}
for name, result in checks.items():
print(f"{name}: {result}")
assert all(checks.values())
states = []
for processed in range(4):
events = list(range(processed))
try:
events[1]
access = "valid"
except IndexError:
access = "trap"
states.append((processed, len(events), access))
print("timeout states:", states)
assert any(access == "trap" for _, _, access in states)
assert states[0][2] == "trap"
assert states[1][2] == "trap"
assert states[2][2] == "valid"
print("result: a timeout can leave report.events[1] out of range")
PYRepository: manaflow-ai/cmux
Length of output: 427
Guard the event count before indexed access.
Change the test to async throws. Use try #require(await log.processedCount() >= 4) after the deadline loop, then require report.events.count == 4 before accessing report.events[1].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swift`
around lines 168 - 181, Update the affected test method in
MobileIrohSettingsModelTests to be async throws, then use try `#require` to verify
log.processedCount() is at least 4 after the deadline loop. After taking the
snapshot, require report.events.count == 4 before accessing report.events[1],
preserving the existing event and failure assertions.
Source: Learnings
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5d0e66d. Configure here.
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
This was referenced Aug 11, 2026
…e-logging # Conflicts: # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/OnboardingFlowView.swift # Sources/PortalSplitDividerCacheInvalidator.swift
This was referenced Aug 12, 2026
This was referenced Sep 23, 2026
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
1 participant
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.
Summary
Testing
recordAppEventcall sites.ioslogmacOS app passed the hosted tagged build, rendered on LG HDR 4K, kept its tagged socket alive, minted a trusted physical-device ticket, and produced no new crash report.cmux-dev-ioslogiOS 27.0 simulator.CmuxMobileShelljob remains red on timing-sensitive failures also reproduced onorigin/main. Both full simulator jobs reached the workflow's 35-minute limit. Focused coverage and the direct archive/build path are green.Demo Video
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.