feat: implement system-shade download progress notifications for Android#453
Merged
rainxchzed merged 5 commits intomainfrom Apr 24, 2026
Merged
feat: implement system-shade download progress notifications for Android#453rainxchzed merged 5 commits intomainfrom
rainxchzed merged 5 commits intomainfrom
Conversation
- Introduce `DownloadProgressNotifier` interface and its Android implementation to surface live download progress in the system notification tray. - Add `DownloadNotificationObserver` to translate `DownloadOrchestrator` state transitions into notification updates with throttling (400ms) to optimize performance. - Implement `DownloadCancelReceiver` to handle download cancellation directly from the notification action. - Add a no-op `DesktopDownloadProgressNotifier` for the JVM target to maintain platform-agnostic common code. - Create a new `app_downloads` notification channel with low importance to ensure non-intrusive progress tracking. - Register necessary components in Android Koin modules and `AndroidManifest.xml`. - Clean up redundant imports in `HomeRoot.kt`.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds a cross-platform download progress notification system: a domain notifier interface, Android and JVM platform implementations (real notifications on Android, no-op on desktop), an observer that reconciles orchestrator state to notifications, a broadcast receiver + manifest entry for cancel actions, and DI/startup wiring. Changes
Sequence Diagram(s)sequenceDiagram
participant Orchestrator as DownloadOrchestrator
participant Observer as DownloadNotificationObserver
participant Notifier as AndroidDownloadProgressNotifier
participant Android as Android System
Orchestrator->>Observer: emit download state map
activate Observer
Observer->>Observer: reconcile changes & throttle per-package updates
alt Queued or Downloading
Observer->>Notifier: notifyProgress(pkg, appName, version, percent?, bytes, total?)
activate Notifier
Notifier->>Notifier: format bytes & build notification (Cancel action)
Notifier->>Android: post/update notification (channel app_downloads)
Android-->>Notifier: posted
Notifier-->>Observer: done
deactivate Notifier
else Transition to terminal/non-progress stage
Observer->>Notifier: clearProgress(pkg)
activate Notifier
Notifier->>Android: cancel notification by id
Android-->>Notifier: dismissed
Notifier-->>Observer: done
deactivate Notifier
end
deactivate Observer
sequenceDiagram
participant User as User
participant Notification as Notification UI
participant Receiver as DownloadCancelReceiver
participant Koin as Koin (DI)
participant Orchestrator as DownloadOrchestrator
User->>Notification: tap "Cancel"
Notification->>Receiver: broadcast Intent (ACTION_CANCEL + packageName)
activate Receiver
Receiver->>Receiver: goAsync()
Receiver->>Koin: getKoin()
activate Koin
Koin-->>Receiver: DownloadOrchestrator + CoroutineScope
deactivate Koin
Receiver->>Orchestrator: launch coroutine -> cancel(packageName)
activate Orchestrator
Orchestrator-->>Receiver: cancel processed
deactivate Orchestrator
Receiver->>Receiver: pending.finish()
deactivate Receiver
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
composeApp/src/androidMain/kotlin/zed/rainxch/githubstore/app/GithubStoreApp.kt (1)
43-47: Start the observer on the shared app scope.
DownloadNotificationObserveris documented as a single long-lived subscriber collected on the app-scoped coroutine scope. Starting it onGithubStoreApp’s private scope creates a second process-wide scope, which makes lifecycle and test behavior diverge from the rest of the app. Prefer resolving the sharedCoroutineScopefrom Koin here.♻️ Proposed change
private fun startDownloadNotificationObserver() { // The observer translates orchestrator state into progress // notifications (see `#373`). Kicked off here so it runs for the // whole process lifetime — no ViewModel is involved. - get<DownloadNotificationObserver>().start(appScope) + get<DownloadNotificationObserver>().start(get()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/androidMain/kotlin/zed/rainxch/githubstore/app/GithubStoreApp.kt` around lines 43 - 47, The startDownloadNotificationObserver currently starts DownloadNotificationObserver using GithubStoreApp's private appScope; change it to resolve and use the shared application CoroutineScope from Koin instead so the observer runs on the single app-wide scope. Locate startDownloadNotificationObserver and replace the call that passes the private appScope to DownloadNotificationObserver.start(...) with a call that first resolves the shared CoroutineScope from Koin (the app-scoped CoroutineScope binding) and passes that shared scope into DownloadNotificationObserver.start(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidDownloadProgressNotifier.kt`:
- Around line 49-60: PendingIntent identity currently relies only on
action/component and requestCode (packageName.hashCode()), which can collide;
update AndroidDownloadProgressNotifier where cancelIntent/cancelPendingIntent
are created so each cancel Intent is uniquely identifiable by adding a data URI
that encodes the package (for example using Uri.parse("package:" + packageName)
or similar) before calling PendingIntent.getBroadcast; keep the existing extras
and flags (DownloadCancelReceiver.ACTION_CANCEL, EXTRA_PACKAGE_NAME,
PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE) but
setIntent.data to the unique URI so the PendingIntent identity is per-package
and not reliant on hashCode alone.
In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/DownloadNotificationObserver.kt`:
- Around line 65-72: The collector launched in
DownloadNotificationObserver.start can die if clearProgress throws and leaves
job non-null, so wrap the collect/reconcile body in a try/catch/finally: catch
Throwable around calls that may throw (especially clearProgress and the two
other clearProgress call sites) and handle/log the error (do not rethrow), and
in finally ensure job = null so future start() calls can resubscribe;
additionally protect notifyProgress/clearProgress invocations inside reconcile
with their own small try/catch blocks to prevent a single notification-manager
failure from cancelling the whole orchestrator.downloads.collect coroutine.
---
Nitpick comments:
In
`@composeApp/src/androidMain/kotlin/zed/rainxch/githubstore/app/GithubStoreApp.kt`:
- Around line 43-47: The startDownloadNotificationObserver currently starts
DownloadNotificationObserver using GithubStoreApp's private appScope; change it
to resolve and use the shared application CoroutineScope from Koin instead so
the observer runs on the single app-wide scope. Locate
startDownloadNotificationObserver and replace the call that passes the private
appScope to DownloadNotificationObserver.start(...) with a call that first
resolves the shared CoroutineScope from Koin (the app-scoped CoroutineScope
binding) and passes that shared scope into
DownloadNotificationObserver.start(...).
🪄 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: c3f55b8e-00c4-403a-8f09-5fabcaa36943
📒 Files selected for processing (10)
composeApp/src/androidMain/AndroidManifest.xmlcomposeApp/src/androidMain/kotlin/zed/rainxch/githubstore/app/GithubStoreApp.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/di/PlatformModule.android.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidDownloadProgressNotifier.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/DownloadCancelReceiver.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/DownloadNotificationObserver.ktcore/data/src/jvmMain/kotlin/zed/rainxch/core/data/di/PlatformModule.jvm.ktcore/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloadProgressNotifier.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/system/DownloadProgressNotifier.ktfeature/home/presentation/src/commonMain/kotlin/zed/rainxch/home/presentation/HomeRoot.kt
💤 Files with no reviewable changes (1)
- feature/home/presentation/src/commonMain/kotlin/zed/rainxch/home/presentation/HomeRoot.kt
- Introduce `isReallyInstalled()` and `hasActualUpdate()` extension functions for `InstalledApp` to correctly handle pending installation states. - Update `DeveloperProfileRepositoryImpl` to treat apps as installed only if they are not in a pending install state, preventing UI leakage of failed/parked downloads. - Refactor `SearchViewModel` and `HomeViewModel` to use the new status checks, ensuring consistency between discovery cards and the details screen. - Fix a synchronization issue where "Installed" badges or update notifications would persist incorrectly after a failed installation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
feature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/repository/DeveloperProfileRepositoryImpl.kt (1)
143-148: Consider calling the domain helper directly instead of re-inlining the predicate.The comment already points at
InstalledApp.isReallyInstalled()as the canonical form of this check, but the expression re-implements it inline. Delegating to the helper removes the risk that the two definitions drift (e.g. if "really installed" later grows another clause such as a non-nullinstalledVersion) and matches the pattern adopted inHomeViewModel/SearchViewModelin this same PR.♻️ Proposed refactor
import zed.rainxch.core.domain.model.Platform import zed.rainxch.core.domain.model.RateLimitException +import zed.rainxch.core.domain.model.isReallyInstalled import zed.rainxch.core.domain.repository.FavouritesRepositoryreturn repo.toDomain( hasReleases = hasReleases, hasInstallableAssets = hasInstallableAssets, - // Treat a row as installed only if the actual install - // completed; a parked-download row left by a failed install - // would otherwise leak "Installed" into the UI (see - // `InstalledApp.isReallyInstalled` for the domain-model - // equivalent of this check). - isInstalled = installedApp != null && !installedApp.isPendingInstall, + isInstalled = installedApp.isReallyInstalled(), isFavorite = isFavorite, latestVersion = latestVersion, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/repository/DeveloperProfileRepositoryImpl.kt` around lines 143 - 148, Replace the inline predicate used to set isInstalled in DeveloperProfileRepositoryImpl with the canonical domain helper: call InstalledApp.isReallyInstalled() on installedApp instead of evaluating "installedApp != null && !installedApp.isPendingInstall"; this keeps logic consistent with HomeViewModel/SearchViewModel and avoids drift if the domain rule changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/home/presentation/src/commonMain/kotlin/zed/rainxch/home/presentation/HomeViewModel.kt`:
- Line 120: The top-level aggregate is using the raw per-repo flag
(`it.value.isUpdateAvailable`) which counts pending-install rows and can drift
from each card's state; change the aggregate to use the same computed predicate
used for per-repo mapping (call the repository-state helper `hasActualUpdate()`
on each mapped value) so replace the `installedMap.any {
it.value.isUpdateAvailable }` check with `installedMap.any {
it.value.hasActualUpdate() }` (ensure you reference the same symbol
`hasActualUpdate()` used at lines ~116–117 and ~366–370 so
HomeState.isUpdateAvailable reflects the per-card update visibility).
---
Nitpick comments:
In
`@feature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/repository/DeveloperProfileRepositoryImpl.kt`:
- Around line 143-148: Replace the inline predicate used to set isInstalled in
DeveloperProfileRepositoryImpl with the canonical domain helper: call
InstalledApp.isReallyInstalled() on installedApp instead of evaluating
"installedApp != null && !installedApp.isPendingInstall"; this keeps logic
consistent with HomeViewModel/SearchViewModel and avoids drift if the domain
rule changes.
🪄 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: c06dd3d4-f370-427c-b077-79ae8e6d22a7
📒 Files selected for processing (4)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/InstalledApp.ktfeature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/repository/DeveloperProfileRepositoryImpl.ktfeature/home/presentation/src/commonMain/kotlin/zed/rainxch/home/presentation/HomeViewModel.ktfeature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt
…me/presentation/HomeViewModel.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Fix a potential `PendingIntent` collision in `AndroidDownloadProgressNotifier` by adding a unique data URI to the cancel action, ensuring distinct identities for concurrent downloads. - Enhance `DownloadNotificationObserver` robustness by wrapping `reconcile` and `clearProgress` calls in try-catch blocks to prevent notification errors from crashing the download flow. - Ensure `DownloadNotificationObserver` can be restarted by resetting its internal job reference to `null` upon completion or cancellation. - Refactor `GithubStoreApp` to use a consistent, shared `CoroutineScope` for the `DownloadNotificationObserver`, aligning its lifecycle with the `DownloadOrchestrator`.
… download-progress-notifier
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.
DownloadProgressNotifierinterface and its Android implementation to surface live download progress in the system notification tray.DownloadNotificationObserverto translateDownloadOrchestratorstate transitions into notification updates with throttling (400ms) to optimize performance.DownloadCancelReceiverto handle download cancellation directly from the notification action.DesktopDownloadProgressNotifierfor the JVM target to maintain platform-agnostic common code.app_downloadsnotification channel with low importance to ensure non-intrusive progress tracking.AndroidManifest.xml.HomeRoot.kt.Summary by CodeRabbit
New Features
Bug Fixes
Maintenance