fix(install): aggressive-oem download reliability (E3 R4)#545
fix(install): aggressive-oem download reliability (E3 R4)#545rainxchzed merged 7 commits intomainfrom
Conversation
… non-expedited (10min cap)
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughIntroduce aggressive OEM detection and battery optimization prompting for reliable background updates. Add platform-specific detector implementations, persist user dismissal state, wire expedited work requests with fallback, render a conditional UI card, and include localized strings and release notes across 13 languages. ChangesBattery Optimization Detection and UI Prompt
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as BatteryOptimizationCard
participant VM as TweaksViewModel
participant Detector as AggressiveOemDetector
participant Repo as TweaksRepository
participant System as Android System (PowerManager/Intents)
User->>UI: Tap "Open settings"
UI->>VM: OnOpenBatteryOptimizationSettings action
VM->>Detector: openBatteryOptimizationSettings()
Detector->>System: ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS / fallback
System-->>Detector: Activity started / failure
Detector-->>VM: success/failure
VM->>Repo: setBatteryOptimizationPromptDismissed(true) (on dismiss)
VM->>VM: evaluateBatteryOptimizationCard() (on resume or re-evaluate)
VM->>Detector: isAggressiveOem(), isBatteryOptimizationIgnored()
VM->>Repo: getBatteryOptimizationPromptDismissed()
VM-->>UI: update state.showBatteryOptimizationCard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/UpdateScheduler.kt (1)
49-59:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove
setInitialDelayorsetExpedited—combining them throwsIllegalArgumentException.WorkManager forbids delaying expedited work requests. Line 52's
.setInitialDelay(1, TimeUnit.MINUTES)and line 58's.setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST)on the sameOneTimeWorkRequestwill cause anIllegalArgumentExceptionto be thrown immediately whenenqueueUniqueWork()executes (line 63–66), crashing the app.Choose one:
- Keep the 1-minute grace period: remove
.setExpedited()and its comment.- Prioritize expedited execution: remove
.setInitialDelay().Option A: Remove expedited (preserve the 1-minute delay)
.setInitialDelay(1, TimeUnit.MINUTES) - // Aggressive-OEM ROMs (Oppo / OnePlus / Realme / Xiaomi) - // throttle generic background work hard. Expedited tier - // gets more headroom; fall back to non-expedited when - // the system has no expedited budget left so the work - // still runs eventually rather than failing outright. - .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) .build()Option B: Remove the delay (preserve expedited)
.setConstraints(constraints) - .setInitialDelay(1, TimeUnit.MINUTES) // Aggressive-OEM ROMs (Oppo / OnePlus / Realme / Xiaomi) // throttle generic background work hard. Expedited tier // gets more headroom; fall back to non-expedited when // the system has no expedited budget left so the work // still runs eventually rather than failing outright. .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) .build()🤖 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 `@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/UpdateScheduler.kt` around lines 49 - 59, The OneTimeWorkRequest built as immediateRequest calls both .setInitialDelay(1, TimeUnit.MINUTES) and .setExpedited(...), which is illegal and throws IllegalArgumentException when enqueueUniqueWork(...) runs; remove the conflicting call to avoid the crash—specifically, delete the .setInitialDelay(1, TimeUnit.MINUTES) line (and its related comment about OEM throttling) from the OneTimeWorkRequestBuilder<UpdateCheckWorker>() in UpdateScheduler.kt so the request remains expedited, keeping the .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) and leaving enqueueUniqueWork(...) unchanged.
🧹 Nitpick comments (4)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/system/AggressiveOemDetector.kt (1)
8-8: 💤 Low valueDocument the return-value semantics of
openBatteryOptimizationSettings().The Boolean return value is unspecified — callers and implementors have to guess whether
truemeans "intent was fired", "user is now exempted", or something else. Given the JVM stub returnsfalseand the Android impl presumably returnstrueon a successful intent launch, a short KDoc would prevent future misinterpretation.📝 Suggested KDoc
- fun openBatteryOptimizationSettings(): Boolean + /** + * Attempts to open the system battery-optimization exemption settings for this app. + * `@return` `true` if the settings intent was successfully launched, `false` otherwise + * (e.g. on Desktop where the concept doesn't apply). + */ + fun openBatteryOptimizationSettings(): Boolean🤖 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 `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/system/AggressiveOemDetector.kt` at line 8, The function openBatteryOptimizationSettings() has unspecified Boolean semantics; add a concise KDoc above its declaration in AggressiveOemDetector.kt that defines what the Boolean represents (e.g., true = intent was successfully launched, false = intent not launched/unsupported), and update/align the JVM stub and Android implementation behaviors to match that documented contract so callers and implementors are unambiguous.core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.kt (1)
116-122: 💤 Low valueKDoc slightly overstates when the flag flips to
true.The comment says the flag flips when the user "either granted the exemption or explicitly dismissed the prompt", but based on the PR description the auto-hide on exemption is driven by re-checking
isBatteryOptimizationIgnored()inonStart, not by writing this flag. Only the "Maybe later" tap should actually writetruehere. The inaccuracy could mislead future implementors into callingsetBatteryOptimizationPromptDismissed(true)on the "granted" path unnecessarily.📝 Suggested KDoc correction
- * One-shot watermark for the battery-optimization prompt on - * aggressive-OEM ROMs (Oppo / OnePlus / Realme / Xiaomi / vivo / - * Honor). `false` until the user has either granted the exemption - * or explicitly dismissed the prompt; flips to `true` afterwards - * and is never re-shown. + * One-shot watermark for the battery-optimization prompt on + * aggressive-OEM ROMs (Oppo / OnePlus / Realme / Xiaomi / vivo / + * Honor). `false` until the user explicitly dismisses the prompt + * ("Maybe later"); flips to `true` afterwards and is never re-shown. + * When the user grants the exemption instead, the card auto-hides via + * [AggressiveOemDetector.isBatteryOptimizationIgnored] without setting + * this flag.🤖 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 `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.kt` around lines 116 - 122, Update the KDoc for the battery-optimization prompt flag to be precise: state that setBatteryOptimizationPromptDismissed(true) is written only when the user explicitly dismisses the prompt (e.g., taps "Maybe later"), and that granting the exemption is handled separately by re-checking isBatteryOptimizationIgnored() (called from onStart) rather than by setting this flag; reference setBatteryOptimizationPromptDismissed, isBatteryOptimizationIgnored, and onStart in the KDoc to guide future implementors.feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt (1)
443-462: ⚡ Quick win
runCatchingaround suspending code swallowsCancellationExceptionWrapping
withTimeoutOrNull(and the DataStoreeditcall inOnDismissBatteryOptimizationCardat lines 934–941) insiderunCatchingcatches allThrowables, includingCancellationException. WhenviewModelScopeis cancelled during ViewModel teardown, this causes:
- The cancellation to be logged as an
error— misleading noise in crash/log pipelines.- Execution to continue past the cancellation point, performing a
_state.updateon a dying ViewModel.This is a known Kotlin coroutines anti-pattern. The fix is to re-throw
CancellationExceptioninsideonFailure:♻️ Proposed fix for
evaluateBatteryOptimizationCardrunCatching { withTimeoutOrNull(BATTERY_OPT_PREF_READ_TIMEOUT_MS) { tweaksRepository.getBatteryOptimizationPromptDismissed().firstOrNull() } }.onFailure { error -> + if (error is CancellationException) throw error logger.error( "TweaksViewModel: failed to read battery-opt dismissed flag", error, ) }.getOrNull() ?: falseApply the same change to the
runCatchingblock at lines 934–941 inOnDismissBatteryOptimizationCard.🤖 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 `@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt` around lines 443 - 462, The runCatching in evaluateBatteryOptimizationCard is catching CancellationException and swallowing coroutine cancellation; update the onFailure handler inside evaluateBatteryOptimizationCard's runCatching so that if the caught Throwable is a CancellationException you re-throw it (i.e., if (t is CancellationException) throw t) and only log non-cancellation errors, and apply the identical change to the runCatching wrapper used in OnDismissBatteryOptimizationCard (the DataStore edit block) so cancellation is propagated instead of logged and execution halted.feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Installation.kt (1)
296-306: ⚡ Quick winConsider wrapping the card in
AnimatedVisibilityfor a smoother transition.Without it, the card snaps in/out instantly, which is visually jarring in a mid-list settings layout — especially after "Maybe later" dismissal.
AnimatedVisibilitywith afadeIn/fadeOut(orexpandVertically/shrinkVertically) keeps the experience consistent with the rest of Material 3 Motion.✨ Proposed change
+import androidx.compose.animation.AnimatedVisibility +import androidx.compose.animation.fadeIn +import androidx.compose.animation.fadeOut +import androidx.compose.animation.expandVertically +import androidx.compose.animation.shrinkVertically - if (state.showBatteryOptimizationCard) { - BatteryOptimizationCard( - onOpenSettings = { - onAction(TweaksAction.OnOpenBatteryOptimizationSettings) - }, - onDismiss = { - onAction(TweaksAction.OnDismissBatteryOptimizationCard) - }, - ) - Spacer(Modifier.height(12.dp)) - } + AnimatedVisibility( + visible = state.showBatteryOptimizationCard, + enter = expandVertically() + fadeIn(), + exit = shrinkVertically() + fadeOut(), + ) { + Column { + BatteryOptimizationCard( + onOpenSettings = { + onAction(TweaksAction.OnOpenBatteryOptimizationSettings) + }, + onDismiss = { + onAction(TweaksAction.OnDismissBatteryOptimizationCard) + }, + ) + Spacer(Modifier.height(12.dp)) + } + }🤖 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 `@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Installation.kt` around lines 296 - 306, Wrap the BatteryOptimizationCard (and its following Spacer) in an AnimatedVisibility whose visible condition is state.showBatteryOptimizationCard to animate the card in/out instead of snapping; use Material motion helpers like fadeIn/fadeOut or expandVertically/shrinkVertically for enter/exit transitions, preserving the existing callbacks (onOpenSettings -> TweaksAction.OnOpenBatteryOptimizationSettings and onDismiss -> TweaksAction.OnDismissBatteryOptimizationCard) and keep the Spacer inside the AnimatedVisibility so spacing animates with the card.
🤖 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
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt`:
- Around line 946-948: The lifecycle resume path doesn't dispatch
TweaksAction.OnReevaluateBatteryOptimizationCard, so the battery card isn't
reevaluated when returning from external settings; modify the DisposableEffect
in TweaksRoot.kt where Lifecycle.Event.ON_RESUME currently dispatches
TweaksAction.OnRefreshCacheSize to also dispatch
TweaksAction.OnReevaluateBatteryOptimizationCard (so the ViewModel's
evaluateBatteryOptimizationCard() runs on resume), ensuring both actions are
sent together in the same resume handler.
---
Outside diff comments:
In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/UpdateScheduler.kt`:
- Around line 49-59: The OneTimeWorkRequest built as immediateRequest calls both
.setInitialDelay(1, TimeUnit.MINUTES) and .setExpedited(...), which is illegal
and throws IllegalArgumentException when enqueueUniqueWork(...) runs; remove the
conflicting call to avoid the crash—specifically, delete the .setInitialDelay(1,
TimeUnit.MINUTES) line (and its related comment about OEM throttling) from the
OneTimeWorkRequestBuilder<UpdateCheckWorker>() in UpdateScheduler.kt so the
request remains expedited, keeping the
.setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) and leaving
enqueueUniqueWork(...) unchanged.
---
Nitpick comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.kt`:
- Around line 116-122: Update the KDoc for the battery-optimization prompt flag
to be precise: state that setBatteryOptimizationPromptDismissed(true) is written
only when the user explicitly dismisses the prompt (e.g., taps "Maybe later"),
and that granting the exemption is handled separately by re-checking
isBatteryOptimizationIgnored() (called from onStart) rather than by setting this
flag; reference setBatteryOptimizationPromptDismissed,
isBatteryOptimizationIgnored, and onStart in the KDoc to guide future
implementors.
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/system/AggressiveOemDetector.kt`:
- Line 8: The function openBatteryOptimizationSettings() has unspecified Boolean
semantics; add a concise KDoc above its declaration in AggressiveOemDetector.kt
that defines what the Boolean represents (e.g., true = intent was successfully
launched, false = intent not launched/unsupported), and update/align the JVM
stub and Android implementation behaviors to match that documented contract so
callers and implementors are unambiguous.
In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Installation.kt`:
- Around line 296-306: Wrap the BatteryOptimizationCard (and its following
Spacer) in an AnimatedVisibility whose visible condition is
state.showBatteryOptimizationCard to animate the card in/out instead of
snapping; use Material motion helpers like fadeIn/fadeOut or
expandVertically/shrinkVertically for enter/exit transitions, preserving the
existing callbacks (onOpenSettings ->
TweaksAction.OnOpenBatteryOptimizationSettings and onDismiss ->
TweaksAction.OnDismissBatteryOptimizationCard) and keep the Spacer inside the
AnimatedVisibility so spacing animates with the card.
In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt`:
- Around line 443-462: The runCatching in evaluateBatteryOptimizationCard is
catching CancellationException and swallowing coroutine cancellation; update the
onFailure handler inside evaluateBatteryOptimizationCard's runCatching so that
if the caught Throwable is a CancellationException you re-throw it (i.e., if (t
is CancellationException) throw t) and only log non-cancellation errors, and
apply the identical change to the runCatching wrapper used in
OnDismissBatteryOptimizationCard (the DataStore edit block) so cancellation is
propagated instead of logged and execution halted.
🪄 Autofix (Beta)
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: CHILL
Plan: Pro
Run ID: 12faa370-3987-44e9-9044-126c3519b30e
📒 Files selected for processing (40)
composeApp/src/androidMain/AndroidManifest.xmlcore/data/src/androidMain/kotlin/zed/rainxch/core/data/di/PlatformModule.android.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidAggressiveOemDetector.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/UpdateScheduler.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/TweaksRepositoryImpl.ktcore/data/src/jvmMain/kotlin/zed/rainxch/core/data/di/PlatformModule.jvm.ktcore/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopAggressiveOemDetector.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/system/AggressiveOemDetector.ktcore/presentation/src/commonMain/composeResources/files/whatsnew/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ar/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/bn/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/es/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/fr/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/hi/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/it/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ja/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ko/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/pl/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ru/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/tr/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/17.jsoncore/presentation/src/commonMain/composeResources/values-ar/strings-ar.xmlcore/presentation/src/commonMain/composeResources/values-bn/strings-bn.xmlcore/presentation/src/commonMain/composeResources/values-es/strings-es.xmlcore/presentation/src/commonMain/composeResources/values-fr/strings-fr.xmlcore/presentation/src/commonMain/composeResources/values-hi/strings-hi.xmlcore/presentation/src/commonMain/composeResources/values-it/strings-it.xmlcore/presentation/src/commonMain/composeResources/values-ja/strings-ja.xmlcore/presentation/src/commonMain/composeResources/values-ko/strings-ko.xmlcore/presentation/src/commonMain/composeResources/values-pl/strings-pl.xmlcore/presentation/src/commonMain/composeResources/values-ru/strings-ru.xmlcore/presentation/src/commonMain/composeResources/values-tr/strings-tr.xmlcore/presentation/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xmlcore/presentation/src/commonMain/composeResources/values/strings.xmlfeature/tweaks/presentation/build.gradle.ktsfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksAction.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksState.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Installation.kt
Foundation Sprint Task 4 / E3 R4. Survey #41: "Fix download issues on Oppo devices (that's one of the reasons I switched to Obtainium)."
Problem
ColorOS / MIUI / Funtouch / MagicOS aggressively kill background WorkManager tasks even when scheduled through GitHub Store's own settings, and intercept system installer dialogs. Result: scheduled update checks silently fail, auto-update via Shizuku doesn't fire, and downloads stall mid-flight.
Three layers of defence shipped in this PR:
1. Aggressive-OEM detector
New
AggressiveOemDetectorinterface (commonMain) + Android impl that flags devices viaBuild.BRAND/Build.MANUFACTURERsubstring match against the known-aggressive list (Oppo, OnePlus, Realme, Xiaomi, Redmi, Poco, vivo, iQOO, Huawei, Honor, Meizu). Desktop returns false / always-ignored — non-issue off-Android. Wired through Koinsinglein both platform modules.2. Tweaks → Updates → "Allow background updates" card
Surfaces only when
(aggressive OEM) && !(already exempted) && !(user dismissed). One tap routes toSettings.ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONSfor the targeted whitelist screen, falls back to the generic optimization-settings screen if the targeted intent is rejected. "Maybe later" persists aBATTERY_OPT_PROMPT_DISMISSED_KEYwatermark so the user is never re-nagged. Card auto-disappears on Tweaks re-entry once the exemption is granted (state re-evaluated inonStart).3. WorkManager priority changes
UpdateCheckWorkerimmediate one-shot now usessetExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST)— short, network-only work, fits comfortably in the 10-minute expedited cap.AutoUpdateWorkerdeliberately stays NON-expedited per reviewer feedback — it downloads multiple APKs and installs sequentially, which can exceed 10 minutes and would get the worker terminated mid-install. The worker already promotes itself to a foreground service viasetForeground, which gives it the headroom it needs without the time cap.4. Manifest
REQUEST_IGNORE_BATTERY_OPTIMIZATIONSpermission added withtools:ignore="BatteryLife". GitHub Store ships outside Play Store (sideloaded APK from GitHub Releases), so the targeted intent is the right tool — Play policy compliance is moot.FOREGROUND_SERVICE_DATA_SYNCalready present (E12) — verified.getExternalFilesDir(DIRECTORY_DOWNLOADS)).Strings
battery_optimization_card_title,_description,_open,_dismiss) shipped across all 13 locales.What's-new
17.json× 13 locales.Test plan
Cannot verify on real device — no Oppo / ColorOS hardware available.
Static checks done:
:composeApp:compileDebugKotlinAndroid✅:composeApp:compileKotlinJvm✅For the operator's device-side smoke pass:
Reviewer notes (CodeRabbit pass — applied + skipped)
withTimeoutOrNullwith explicit logger.error on failure; duplicate evaluate-on-onStart removed.REQUEST_IGNORE_BATTERY_OPTIMIZATIONSPlay-policy concern (rationale above — sideloaded APK, not a Play app); pre-existingapi(libs.ktor.client.core)infeature/tweaks/presentation/build.gradle.kts(not in scope of this PR);RELEASE_NOTES_1.8.1.mdgrammar nit (untracked working file, not in diff).Summary by CodeRabbit
New Features
Improvements
Chores