Skip to content

Move mobile telemetry consent into CMUXMobileCore - #9505

Merged
austinywang merged 2 commits into
mainfrom
issue-7724-move-analyticsconsentproviding-to-cmuxmo
Aug 4, 2026
Merged

austinywang merged 2 commits into
mainfrom
issue-7724-move-analyticsconsentproviding-to-cmuxmo

Conversation

@austinywang

@austinywang austinywang commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #7724

Summary

  • move AnalyticsConsentProviding and UserDefaultsAnalyticsConsentProvider into CMUXMobileCore
  • make analytics and crash reporting sibling consumers of the Core consent contract
  • remove CmuxMobileCrashReporting's dependency on CmuxMobileAnalytics and keep the analytics-only closure adapter in its own file

Testing

  • Red, test-only commit 5b5600698c: iOS package run failed in CMUXMobileCoreTests with cannot find 'UserDefaultsAnalyticsConsentProvider' in scope
  • Green, fix commit 80602f5195: hosted Core package step passed swift test --package-path Packages/Shared/CMUXMobileCore
  • Fleet Mac (cmux12s-mac-mini):
    • swift test --package-path Packages/Shared/CMUXMobileCore --filter UserDefaultsAnalyticsConsentProviderTests — 1 test passed
    • swift test --package-path Packages/iOS/CmuxMobileAnalytics — 27 tests passed
    • swift test --package-path Packages/iOS/CmuxMobileCrashReporting — 17 tests passed
    • resolved crash-reporting dependencies were cmuxmobilecore, cmuxsentrytelemetry, and sentry-cocoa; no analytics dependency remained
  • python3 scripts/check-package-resolved-policy.py
  • python3 scripts/check-workspace-package-groups.py --check
  • ./scripts/reload-cloud.sh --tag sym7724 — Blacksmith build succeeded; the build was not launched because this issue has no socket-visible runtime behavior, and cleanup confirmed no tagged process or debug socket remained

The manual iOS workflow's repository-wide conventions lane is independently red on both commits for existing main-branch violations; the targeted Core package step provides the red-to-green regression signal above.

Summary by CodeRabbit

  • New Features

    • Added a shared analytics consent interface for telemetry and crash-reporting opt-out checks.
    • Added consent providers backed by app settings or dynamically evaluated closures.
    • Consent changes are reflected immediately, while missing or invalid settings default to disabled.
  • Documentation

    • Added guidance for configuring and testing telemetry consent with isolated settings.
  • Tests

    • Added coverage for default-disabled behavior and live consent changes.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request moves shared analytics consent types into CMUXMobileCore. Analytics retains a closure-backed provider, and crash reporting changes its dependency and imports to use the shared contract.

Changes

Shared consent contract

Layer / File(s) Summary
Core consent providers
Packages/Shared/CMUXMobileCore/..., Packages/Shared/CMUXMobileCore/Tests/...
Adds AnalyticsConsentProviding and UserDefaultsAnalyticsConsentProvider. Tests verify disabled defaults and live preference changes. Documentation describes the contract and isolated test setup.
Analytics consent integration
Packages/iOS/CmuxMobileAnalytics/Package.swift, Packages/iOS/CmuxMobileAnalytics/Sources/..., Packages/iOS/CmuxMobileAnalytics/Tests/...
Adds the closure-backed AnalyticsConsentProvider and updates analytics tests to import CMUXMobileCore. The previous consent declarations file is removed.
Crash reporting dependency migration
Packages/iOS/CmuxMobileCrashReporting/Package.swift, Packages/iOS/CmuxMobileCrashReporting/Sources/..., Packages/iOS/CmuxMobileCrashReporting/Tests/...
Replaces the CmuxMobileAnalytics consent dependency with CMUXMobileCore in package configuration, source imports, and tests.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • manaflow-ai/cmux#9305: Centralizes consent-aware telemetry and Sentry reporting through AnalyticsConsentProviding.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR satisfies issue #7724 by centralizing the consent contract and removing the crash-reporting dependency on analytics.
Out of Scope Changes check ✅ Passed All changes support the consent-contract move, dependency updates, tests, or related documentation; no unrelated code is present.
Cmux Swift Actor Isolation ✅ Passed The diff adds no implicit MainActor isolation: the Core protocol is synchronous, packages have no default MainActor setting, and the moved provider retains documented nonisolated(unsafe) storage.
Cmux Swift Blocking Runtime ✅ Passed The full PR diff adds no semaphore, wait, sleep, delayed-dispatch, polling, main-queue sync, or manual-lock primitive; the crash watcher queue code is unchanged and test scaffolding is deterministic.
Cmux Browser Automation Off-Main ✅ Passed PASS: The patch changes only shared Core and iOS consent files; it does not touch browser automation targets or introduce browser/WebKit/socket-worker commands.
Cmux Expensive Synchronous Load ✅ Passed The diff only moves consent types and adds a closure adapter; no agent-history loader, large-file parsing, directory scan, or interactive synchronous load was added or moved.
Cmux Cache Substitution Correctness ✅ Passed The diff only relocates consent providers; UserDefaults still reads object(forKey:) on every access, the closure runs per read, and no persistence/history/undo/snapshot cache substitution appears.
Cmux No Hacky Sleeps ✅ Passed The diff changes only Swift, Swift package manifests, tests, and Markdown; no covered non-Swift runtime path or added sleep, timer, polling, or fixed-delay synchronization was found.
Cmux Algorithmic Complexity ✅ Passed The production patch adds only O(1) consent reads and a closure call, plus imports and dependency moves; it adds no scalable collection scans or nested loops.
Cmux Swift Concurrency ✅ Passed The PR adds only synchronous consent APIs; added Swift lines contain no legacy Dispatch, Task, Combine, or completion patterns. Existing queues remain unchanged and only imports moved.
Cmux Swift @Concurrent ✅ Passed The commit adds no async functions or async call sites and no @concurrent changes; it only moves synchronous consent code and its existing nonisolated(unsafe) UserDefaults field.
Cmux Swift Package Boundaries ✅ Passed The diff moves reusable consent protocol and UserDefaults provider into CMUXMobileCore, a small SwiftPM target; analytics and crash reporting consume the shared contract.
Cmux Swiftpm Lockfiles ✅ Passed Manifest edits only add or replace local package dependencies; no external pins changed. CrashReporting/Package.resolved is unchanged, with no Xcode package-reference or .gitignore diff.
Cmux Swift Logging ✅ Passed The diff adds or changes no print, debugPrint, dump, NSLog, Logger, or ad hoc file/stdout logging; it only moves consent code and updates imports and documentation.
Cmux User-Facing Error Privacy ✅ Passed The diff adds no user-facing errors, alerts, command output, API error bodies, or recovery copy; changes are consent APIs, imports, comments, tests, and documentation.
Cmux Full Internationalization ✅ Passed The diff adds only developer-facing Swift API docs, comments, test code, configuration tokens, and a package README; it adds no user-facing copy, UI text, or localization/catalog changes.
Cmux Swiftui State Layout ✅ Passed The diff contains no SwiftUI views or state/layout constructs; it only changes Foundation-based consent providers, package manifests, tests, and imports.
Cmux Architecture Rethink ✅ Passed The diff moves the consent contract to CMUXMobileCore and removes the crash-reporting analytics dependency; added code introduces no timing, lock, observer, side-channel, or UI-lifecycle workaround.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The Swift diff only moves consent providers and updates package imports/tests; it adds or changes no NSWindow, NSPanel, Window, WindowGroup, or close-shortcut code.
Cmux Source Artifacts ✅ Passed The commit changes only Swift source, tests, package manifests, and a README; the removed Swift file is an artifact removal, and no artifact-like path or binary output was added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Changed production Swift adds the real consent contract/providers only; no DEBUG/test guard or debug/ForTesting/TestHook/TestSeam member was added, and crash DEBUG behavior was only touched inciden...
Cmux No Ambient Global State ✅ Passed Added Swift code uses constructable, injectable structs; the only static member is the allowed telemetryKey constant, with no new top-level functions, mutable globals, static namespaces, or runtime...
Title check ✅ Passed The title clearly and concisely describes moving mobile telemetry consent into the shared CMUXMobileCore package.
Description check ✅ Passed The description includes a detailed summary, testing results, dependency verification, and build validation; the demo video is not needed for this package refactor.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-7724-move-analyticsconsentproviding-to-cmuxmo

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.

❤️ Share

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

@austinywang
austinywang merged commit 7693b19 into main Aug 4, 2026
8 of 12 checks passed
@austinywang
austinywang deleted the issue-7724-move-analyticsconsentproviding-to-cmuxmo branch August 4, 2026 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move AnalyticsConsentProviding to CMUXMobileCore so crash reporting and analytics are sibling consumers

1 participant