feat(details): Implement open, uninstall, and downgrade functionality#253
feat(details): Implement open, uninstall, and downgrade functionality#253rainxchzed merged 8 commits intomainfrom
Conversation
This commit introduces the ability for users to open and uninstall applications directly from the details screen. It also adds a warning mechanism for downgrades, requiring users to uninstall the current version before installing an older one. - **feat(details)**: Added `Open` and `Uninstall` buttons and their corresponding logic to the app details screen. - **feat(android)**: Implemented `openApp` and `uninstall` functionality in `AndroidInstaller` using Android's `Intent` system. - **feat(i18n)**: Added new string resources for uninstall, open, and downgrade warning dialogs. - **refactor(details)**: Introduced a `ShowDowngradeWarning` event to inform the user when an installation would result in a downgrade, prompting them to uninstall the existing version first. - **chore(desktop)**: Provided stub implementations for `openApp` and `uninstall` on desktop, as these actions are not supported in the same way.
This commit introduces `PackageEventReceiver`, a `BroadcastReceiver` that listens for system-wide package installation, update, and removal events (`ACTION_PACKAGE_ADDED`, `ACTION_PACKAGE_REPLACED`, `ACTION_PACKAGE_FULLY_REMOVED`). This receiver improves data consistency by reacting to out-of-app package changes. When an app is installed or updated, it updates its record in the local database with the correct version information from the system and clears any "pending install" flags. If an app is uninstalled, its corresponding entry is removed from the database. - **feat(data)**: Created `PackageEventReceiver.kt` to handle package-related broadcast intents. - **refactor(data)**: Moved package event logic into a dedicated receiver to centralize state management for installed applications.
This commit introduces a downgrade warning mechanism on Android and adds new user actions for opening and uninstalling applications directly from the details screen. A check is now performed before installation to detect potential downgrades. If the selected version is different from the installed version and is not marked as an update, a warning is displayed to the user. Additionally, new actions have been implemented to allow users to open or uninstall the currently viewed application. - **feat(android)**: Added a downgrade warning that triggers when a user attempts to install an older version of an app. - **feat(details)**: Implemented `OpenApp` and `UninstallApp` actions in `DetailsViewModel`. - **refactor(details)**: Introduced a `normalizeVersion` utility function to improve version comparison logic.
|
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:
WalkthroughAdds uninstall and open-app APIs and implementations across platforms, a PackageEventReceiver to handle package install/remove broadcasts, Flow-based installed-app observation, UI actions/events and ViewModel flows for uninstall/open and downgrade warnings, string resources, manifest permission updates, and minor downloader/installer adjustments. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as App UI (Details / Apps)
participant VM as ViewModel (DetailsViewModel / AppsViewModel)
participant Installer as Installer (Android)
participant System as Android System
participant BR as PackageEventReceiver
participant Repo as InstalledAppsRepository
User->>UI: Tap Open / Uninstall / Install
UI->>VM: Dispatch Action (OpenApp / UninstallApp / InstallPrimary)
alt OpenApp
VM->>Installer: openApp(packageName)
Installer->>System: startActivity(launch intent)
alt launch success
System->>User: App launches
else launch failure
VM->>UI: Emit failed_to_open_app
end
else Uninstall flow
VM->>Installer: uninstall(packageName)
Installer->>System: startActivity(Intent.ACTION_DELETE)
System->>BR: Broadcast PACKAGE_FULLY_REMOVED
BR->>Repo: deleteApp(packageName)
Repo-->>BR: ack/update
BR->>VM: notify installed-app change
else InstallPrimary triggers downgrade detection
VM->>VM: Emit ShowDowngradeWarning
VM->>UI: Show dialog
User->>UI: Confirm uninstall
UI->>VM: Dispatch UninstallApp
VM->>Installer: uninstall(packageName)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
This commit introduces the ability for users to uninstall applications directly from the "Apps" screen. An "Uninstall" button is now displayed on each application card. Tapping this button initiates the uninstallation process for the corresponding app. This functionality is also integrated with the app details screen, which will now prompt the user with a downgrade warning, offering to uninstall the current version before proceeding. - **feat(apps)**: Added an uninstall icon button to `AppCard` to trigger uninstallation. - **feat(apps)**: Implemented `OnUninstallApp` action and corresponding logic in `AppsViewModel` to handle the uninstall request via the `installer`. - **feat(details)**: Added a confirmation dialog to the details screen to handle downgrades, which require uninstalling the app first.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt (2)
84-92: Unused variableappinonPackageRemoved.
appis fetched on line 86 but only used for the null check. The deletion on line 87 usespackageNamedirectly. You can simplify with a null-check call or useappfor the delete.Proposed simplification
private suspend fun onPackageRemoved(packageName: String) { try { - val app = installedAppsRepository.getAppByPackage(packageName) ?: return + installedAppsRepository.getAppByPackage(packageName) ?: return installedAppsRepository.deleteInstalledApp(packageName) Logger.i { "Removed uninstalled app via broadcast: $packageName" } } catch (e: Exception) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt` around lines 84 - 92, The local variable app in onPackageRemoved is unused; replace "val app = installedAppsRepository.getAppByPackage(packageName) ?: return" with a direct null-check call like "installedAppsRepository.getAppByPackage(packageName) ?: return" (or use app for the subsequent delete if you need fields from it), keeping the installedAppsRepository.deleteInstalledApp(packageName) and the log/error handling intact.
26-26: Consider adding lifecycle management for the CoroutineScope to prevent potential leaks if the receiver is ever unregistered.Currently,
PackageEventReceivercreates an internalCoroutineScopewith no way to cancel it. While the receiver is registered for the app's lifetime in the current implementation, this design is fragile—if unregistration is added later, in-flight coroutines would continue running. Expose acancel()method or accept an external scope to ensure proper cleanup.Additionally, in
onPackageRemoved()(line 86), theappvariable is retrieved but only the null-check matters; the actual deletion uses onlypackageName. Simplify by removing the unused binding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt` at line 26, PackageEventReceiver creates an internal CoroutineScope (private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)) with no cancellation — add lifecycle management by either accepting a CoroutineScope in the constructor or exposing a cancel/close method that calls scope.cancel() so in-flight coroutines are cleaned up when the receiver is unregistered; also simplify onPackageRemoved by removing the unused local app binding (only packageName is needed) and only perform the deletion using packageName to avoid the unused variable.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt (1)
958-960:normalizeVersionis duplicated in three places.This exact function exists in
SmartInstallButton.kt(line 447) andInstalledAppsRepositoryImpl.kt(line 224). Consider extracting it to a shared utility (e.g., in the domain or core presentation layer) to keep it DRY.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 958 - 960, The normalizeVersion function is duplicated across DetailsViewModel, SmartInstallButton, and InstalledAppsRepositoryImpl; extract it to a shared utility in the domain or core presentation layer (e.g., VersionUtils or VersionNormalizer) and replace the local implementations with calls to that single function; update DetailsViewModel.normalizeVersion, SmartInstallButton (the function at line ~447), and InstalledAppsRepositoryImpl (the function at line ~224) to use the new shared function so all three locations delegate to the common implementation.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt (2)
67-71: RedundantinstalledApp != nullguard
isInstalledis already defined asinstalledApp != null && !installedApp.isPendingInstall, so the secondinstalledApp != null &&on line 68 is unreachable-false. Kotlin's smart-cast also flows through after theisInstalledcheck.♻️ Proposed simplification
- val isSameVersionInstalled = isInstalled && - installedApp != null && - normalizeVersion(installedApp.installedVersion) == normalizeVersion( - state.selectedRelease?.tagName ?: "" - ) + val isSameVersionInstalled = isInstalled && + normalizeVersion(installedApp!!.installedVersion) == normalizeVersion( + state.selectedRelease?.tagName ?: "" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt` around lines 67 - 71, The check inside isSameVersionInstalled redundantly tests installedApp != null after isInstalled already implies installedApp != null; remove the extra installedApp != null condition so isSameVersionInstalled uses isInstalled && normalizeVersion(installedApp.installedVersion) == normalizeVersion(state.selectedRelease?.tagName ?: "") allowing Kotlin's smart-cast on installedApp to apply and simplifying the expression.
448-450:normalizeVersionis triplicated — extract to a shared utilityThe exact same function exists in three places:
SmartInstallButton.kt(this file, line 448)DetailsViewModel.kt(line 957)InstalledAppsRepositoryImpl.kt(line 224)Any future change to version-prefix normalization rules must be applied in all three. Move this to a single shared location (e.g.,
core/domain/src/.../util/VersionUtils.kt) and have all callers import it from there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt` around lines 448 - 450, Extract the duplicated normalizeVersion logic into a single shared utility function (e.g., fun normalizeVersion(version: String): String) in a new VersionUtils (or VersionUtil) file in the core/domain/shared utilities module, then replace the local implementations in SmartInstallButton, DetailsViewModel, and InstalledAppsRepositoryImpl with imports that call the shared normalizeVersion; ensure the new utility preserves current behavior (removePrefix "v"/"V" and trim) and update imports in all three callers to reference the centralized function.
🤖 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/AndroidInstaller.kt`:
- Around line 170-180: In openApp override (function openApp) wrap the
context.startActivity(launchIntent) call in a try-catch that catches
ActivityNotFoundException (and optionally Exception for safety), log a warning
via Logger.w including the packageName and exception message/stack, and return
false when the launch fails; keep the existing flow of adding
Intent.FLAG_ACTIVITY_NEW_TASK and returning true only on successful start.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`:
- Around line 87-125: Replace the Modifier.clickable usage on the ElevatedCard
with the ElevatedCard(onClick = { ... }) overload so accessibility
semantics/ripple/enabled state are applied; specifically move the click lambda
that calls onAction(DetailsAction.UninstallApp) into ElevatedCard(onClick = {
onAction(DetailsAction.UninstallApp) }) and remove the Modifier.clickable from
the modifier chain (apply the same change for the Open card variant that calls
onAction(DetailsAction.OpenApp)). Ensure other modifiers (weight, height,
liquefiable) remain on the same Modifier chain and keep the card colors/shape
unchanged.
- Around line 181-184: The UI may show the literal "null" because
installedApp?.latestVersion.toString() force-converts null; update the
isUpdateAvailable branch to guard latestVersion and provide a safe default, e.g.
use installedApp?.latestVersion?.toString() ?: "" (or another placeholder like
"—") instead of installedApp?.latestVersion.toString(); locate this in
SmartInstallButton.kt inside the isUpdateAvailable ->
stringResource(Res.string.update_to_version, ...) expression and replace the
argument with a null-safe expression so null latestVersion never renders as
"null".
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 468-478: The getString call is passing installedApp.appName
wrapped in arrayOf(), which sends an Array<String> instead of a String and
breaks formatting; in DetailsViewModel inside the viewModelScope.launch where
_events.send(DetailsEvent.OnMessage(...)) is built, remove arrayOf(...) and pass
installedApp.appName directly to getString(Res.string.failed_to_open_app,
installedApp.appName) so the %1$s placeholder receives the actual app name.
- Around line 304-322: The downgrade-warning logic in DetailsViewModel
incorrectly treats any normalized-version mismatch as a downgrade (causing false
positives when a user manually selects a newer release) — update the check in
the block that builds and sends DetailsEvent.ShowDowngradeWarning to (a) prefer
comparing numeric version codes when available (use
InstalledApp.installedVersionCode and InstalledApp.latestVersionCode if present)
and only treat as a downgrade when installedVersionCode > targetVersionCode, and
(b) if version codes are not yet available, defer emitting the downgrade warning
until after asset installation/version-code extraction rather than relying
solely on normalizeVersion(release.tagName); additionally, fix the string
formatting call that currently passes an Array by replacing
getString(Res.string.failed_to_open_app, arrayOf(installedApp.appName)) with
getString(Res.string.failed_to_open_app, installedApp.appName) so the appName is
passed as the format argument.
---
Nitpick comments:
In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt`:
- Around line 84-92: The local variable app in onPackageRemoved is unused;
replace "val app = installedAppsRepository.getAppByPackage(packageName) ?:
return" with a direct null-check call like
"installedAppsRepository.getAppByPackage(packageName) ?: return" (or use app for
the subsequent delete if you need fields from it), keeping the
installedAppsRepository.deleteInstalledApp(packageName) and the log/error
handling intact.
- Line 26: PackageEventReceiver creates an internal CoroutineScope (private val
scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)) with no cancellation —
add lifecycle management by either accepting a CoroutineScope in the constructor
or exposing a cancel/close method that calls scope.cancel() so in-flight
coroutines are cleaned up when the receiver is unregistered; also simplify
onPackageRemoved by removing the unused local app binding (only packageName is
needed) and only perform the deletion using packageName to avoid the unused
variable.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`:
- Around line 67-71: The check inside isSameVersionInstalled redundantly tests
installedApp != null after isInstalled already implies installedApp != null;
remove the extra installedApp != null condition so isSameVersionInstalled uses
isInstalled && normalizeVersion(installedApp.installedVersion) ==
normalizeVersion(state.selectedRelease?.tagName ?: "") allowing Kotlin's
smart-cast on installedApp to apply and simplifying the expression.
- Around line 448-450: Extract the duplicated normalizeVersion logic into a
single shared utility function (e.g., fun normalizeVersion(version: String):
String) in a new VersionUtils (or VersionUtil) file in the core/domain/shared
utilities module, then replace the local implementations in SmartInstallButton,
DetailsViewModel, and InstalledAppsRepositoryImpl with imports that call the
shared normalizeVersion; ensure the new utility preserves current behavior
(removePrefix "v"/"V" and trim) and update imports in all three callers to
reference the centralized function.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 958-960: The normalizeVersion function is duplicated across
DetailsViewModel, SmartInstallButton, and InstalledAppsRepositoryImpl; extract
it to a shared utility in the domain or core presentation layer (e.g.,
VersionUtils or VersionNormalizer) and replace the local implementations with
calls to that single function; update DetailsViewModel.normalizeVersion,
SmartInstallButton (the function at line ~447), and InstalledAppsRepositoryImpl
(the function at line ~224) to use the new shared function so all three
locations delegate to the common implementation.
core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidInstaller.kt
Show resolved
Hide resolved
| ElevatedCard( | ||
| modifier = Modifier | ||
| .weight(1f) | ||
| .height(52.dp) | ||
| .clickable { onAction(DetailsAction.UninstallApp) } | ||
| .liquefiable(liquidState), | ||
| colors = CardDefaults.elevatedCardColors( | ||
| containerColor = MaterialTheme.colorScheme.errorContainer | ||
| ), | ||
| shape = RoundedCornerShape( | ||
| topStart = 24.dp, | ||
| bottomStart = 24.dp, | ||
| topEnd = 6.dp, | ||
| bottomEnd = 6.dp | ||
| ) | ||
| ) { | ||
| Box( | ||
| modifier = Modifier.fillMaxSize(), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Row( | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| horizontalArrangement = Arrangement.spacedBy(6.dp) | ||
| ) { | ||
| Icon( | ||
| imageVector = Icons.Default.Delete, | ||
| contentDescription = null, | ||
| modifier = Modifier.size(18.dp), | ||
| tint = MaterialTheme.colorScheme.onErrorContainer | ||
| ) | ||
| Text( | ||
| text = stringResource(Res.string.uninstall), | ||
| color = MaterialTheme.colorScheme.onErrorContainer, | ||
| fontWeight = FontWeight.Bold, | ||
| style = MaterialTheme.typography.titleMedium | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Use ElevatedCard(onClick = …) for proper accessibility semantics
Both cards attach click via Modifier.clickable, which does not set Role.Button in the accessibility tree. ElevatedCard(onClick = { … }) is the Material 3 idiomatic API that wires the ripple, enabled state, and Role.Button semantics automatically.
♻️ Proposed fix (Uninstall card — same pattern for Open card)
- ElevatedCard(
- modifier = Modifier
- .weight(1f)
- .height(52.dp)
- .clickable { onAction(DetailsAction.UninstallApp) }
- .liquefiable(liquidState),
- colors = CardDefaults.elevatedCardColors(
- containerColor = MaterialTheme.colorScheme.errorContainer
- ),
- shape = RoundedCornerShape(
- topStart = 24.dp,
- bottomStart = 24.dp,
- topEnd = 6.dp,
- bottomEnd = 6.dp
- )
- ) {
+ ElevatedCard(
+ onClick = { onAction(DetailsAction.UninstallApp) },
+ modifier = Modifier
+ .weight(1f)
+ .height(52.dp)
+ .liquefiable(liquidState),
+ colors = CardDefaults.elevatedCardColors(
+ containerColor = MaterialTheme.colorScheme.errorContainer
+ ),
+ shape = RoundedCornerShape(
+ topStart = 24.dp,
+ bottomStart = 24.dp,
+ topEnd = 6.dp,
+ bottomEnd = 6.dp
+ )
+ ) {Also applies to: 128-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`
around lines 87 - 125, Replace the Modifier.clickable usage on the ElevatedCard
with the ElevatedCard(onClick = { ... }) overload so accessibility
semantics/ripple/enabled state are applied; specifically move the click lambda
that calls onAction(DetailsAction.UninstallApp) into ElevatedCard(onClick = {
onAction(DetailsAction.UninstallApp) }) and remove the Modifier.clickable from
the modifier chain (apply the same change for the Open card variant that calls
onAction(DetailsAction.OpenApp)). Ensure other modifiers (weight, height,
liquefiable) remain on the same Modifier chain and keep the card colors/shape
unchanged.
| isUpdateAvailable -> stringResource( | ||
| Res.string.update_to_version, | ||
| installedApp.latestVersion.toString() | ||
| installedApp?.latestVersion.toString() | ||
| ) |
There was a problem hiding this comment.
installedApp?.latestVersion.toString() can produce the literal string "null" in the UI
installedApp?.latestVersion is a nullable expression. Calling .toString() directly (not ?.toString()) coerces null to "null", which would appear verbatim in the update button label. Even though isUpdateAvailable == true guarantees installedApp is non-null, latestVersion itself could be null (e.g., a not-yet-loaded state).
🛡️ Proposed fix
- installedApp?.latestVersion.toString()
+ installedApp?.latestVersion?.toString() ?: ""📝 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.
| isUpdateAvailable -> stringResource( | |
| Res.string.update_to_version, | |
| installedApp.latestVersion.toString() | |
| installedApp?.latestVersion.toString() | |
| ) | |
| isUpdateAvailable -> stringResource( | |
| Res.string.update_to_version, | |
| installedApp?.latestVersion?.toString() ?: "" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`
around lines 181 - 184, The UI may show the literal "null" because
installedApp?.latestVersion.toString() force-converts null; update the
isUpdateAvailable branch to guard latestVersion and provide a safe default, e.g.
use installedApp?.latestVersion?.toString() ?: "" (or another placeholder like
"—") instead of installedApp?.latestVersion.toString(); locate this in
SmartInstallButton.kt inside the isUpdateAvailable ->
stringResource(Res.string.update_to_version, ...) expression and replace the
argument with a null-safe expression so null latestVersion never renders as
"null".
| // Downgrade detection: if app is installed, selected version differs, | ||
| // and it's NOT flagged as an update, it might be a downgrade | ||
| if (installedApp != null && | ||
| !installedApp.isPendingInstall && | ||
| !installedApp.isUpdateAvailable && | ||
| normalizeVersion(release.tagName) != normalizeVersion(installedApp.installedVersion) && | ||
| platform == Platform.ANDROID | ||
| ) { | ||
| viewModelScope.launch { | ||
| _events.send( | ||
| DetailsEvent.ShowDowngradeWarning( | ||
| packageName = installedApp.packageName, | ||
| currentVersion = installedApp.installedVersion, | ||
| targetVersion = release.tagName | ||
| ) | ||
| ) | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find InstalledApp data class definition
ast-grep --pattern 'data class InstalledApp($$$)'Repository: rainxchzed/Github-Store
Length of output: 3757
🏁 Script executed:
# Look for the DetailsViewModel file to understand the context better
fd "DetailsViewModel.kt" --exec head -50 {} \;Repository: rainxchzed/Github-Store
Length of output: 2269
🏁 Script executed:
# Search for how getString is used in the file to understand the API pattern
rg "getString\(" --type kt -A 2 | head -50Repository: rainxchzed/Github-Store
Length of output: 93
🏁 Script executed:
# Search for getString usage patterns with format args in Kotlin files
rg "getString.*arrayOf" --type kotlin -B 2 -A 2Repository: rainxchzed/Github-Store
Length of output: 49
🏁 Script executed:
# Search for GithubRelease model to check available fields
ast-grep --pattern 'data class GithubRelease($$$)'Repository: rainxchzed/Github-Store
Length of output: 1486
🏁 Script executed:
# Look at the DetailsViewModel around line 471-474 to see getString usage
sed -n '468,480p' "feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt"Repository: rainxchzed/Github-Store
Length of output: 521
🏁 Script executed:
# Check how version comparison is done elsewhere in the codebase
rg "versionCode" --type kotlin -B 2 -A 2 | head -60Repository: rainxchzed/Github-Store
Length of output: 7628
🏁 Script executed:
# Check how getString is used elsewhere in DetailsViewModel
sed -n '1,100p' "feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt" | grep -A 3 "getString"Repository: rainxchzed/Github-Store
Length of output: 271
🏁 Script executed:
# Look at more getString examples in the codebase
rg "getString\(Res\.string\." --type kotlin -B 1 -A 3 | head -80Repository: rainxchzed/Github-Store
Length of output: 10174
🏁 Script executed:
# Check the actual downgrade detection logic around line 304-322
sed -n '300,325p' "feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt"Repository: rainxchzed/Github-Store
Length of output: 1394
🏁 Script executed:
# Search for how update availability is determined and set
rg "isUpdateAvailable" --type kotlin -B 3 -A 3 | head -80Repository: rainxchzed/Github-Store
Length of output: 11069
🏁 Script executed:
# Check the Compose Resources getString signature and how it's used with format args
rg "getString\(Res\.string\.[^)]*," --type kotlin | head -20Repository: rainxchzed/Github-Store
Length of output: 1006
🏁 Script executed:
# Look at the actual structure of how getString is supposed to be called
rg "fun getString\(" --type kotlin -B 3 -A 5Repository: rainxchzed/Github-Store
Length of output: 49
🏁 Script executed:
# Check if there's a version code available for the selected release anywhere
sed -n '1,350p' "feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt" | grep -A 20 "InstallPrimary"Repository: rainxchzed/Github-Store
Length of output: 1293
🏁 Script executed:
# Search for how versions are compared elsewhere for update checks
rg "latestVersionCode\|installedVersionCode" --type kotlin -B 3 -A 3 | head -100Repository: rainxchzed/Github-Store
Length of output: 49
🏁 Script executed:
# Look at TimeFormatters to see the getString function signature/usage
cat "core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/utils/TimeFormatters.kt" | head -80Repository: rainxchzed/Github-Store
Length of output: 2906
🏁 Script executed:
# Check what exactly normalizeVersion does in context
sed -n '950,970p' "feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt"Repository: rainxchzed/Github-Store
Length of output: 514
🏁 Script executed:
# Search for version code comparison logic in update detection
rg "installedVersionCode.*latestVersionCode\|latestVersionCode.*installedVersionCode" --type kotlin -B 5 -A 5Repository: rainxchzed/Github-Store
Length of output: 49
🏁 Script executed:
# Check if there's any semver or version comparison utility in the codebase
fd -t f ".*[Vv]ersion.*" --type f -path "*/core/*" | grep -i "util\|helper\|extension" | head -10Repository: rainxchzed/Github-Store
Length of output: 235
🏁 Script executed:
# Look for how update check compares versions and sets isUpdateAvailable
rg "isUpdateAvailable\s*=" --type kotlin -B 5 -A 2 | head -100Repository: rainxchzed/Github-Store
Length of output: 12905
🏁 Script executed:
# Search for version code extraction during install
rg "versionCode\|versionName" --type kotlin -A 3 -B 3 | grep -A 5 -B 5 "saveInstalledApp\|UpdateCheck\|update"Repository: rainxchzed/Github-Store
Length of output: 49
🏁 Script executed:
# Check DetailsEvent.ShowDowngradeWarning to see what parameters it expects
rg "ShowDowngradeWarning" --type kotlin -B 2 -A 8Repository: rainxchzed/Github-Store
Length of output: 2838
🏁 Script executed:
# Verify how the release object is populated and if it has version code info
rg "selectedRelease\s*=" --type kotlin -B 3 -A 10 | head -60Repository: rainxchzed/Github-Store
Length of output: 8658
🏁 Script executed:
# Check if we can extract or access version code for the selected release
sed -n '140,200p' "feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt"Repository: rainxchzed/Github-Store
Length of output: 2336
🏁 Script executed:
# Look at SyncInstalledAppsUseCase to see the complete version comparison logic
rg "latestVersionCode.*systemInfo.versionCode\|systemInfo.versionCode.*latestVersionCode" --type kotlin -B 10 -A 5Repository: rainxchzed/Github-Store
Length of output: 49
🏁 Script executed:
# Look at the complete InstalledAppsRepositoryImpl update check logic
sed -n '200,250p' "core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt"Repository: rainxchzed/Github-Store
Length of output: 1199
🏁 Script executed:
# Check the full SyncInstalledAppsUseCase to understand version code comparison
sed -n '1,100p' "core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt"Repository: rainxchzed/Github-Store
Length of output: 4513
🏁 Script executed:
# Look for how SelectRelease action works to understand when selectedRelease changes
rg "SelectRelease" --type kotlin -B 3 -A 10Repository: rainxchzed/Github-Store
Length of output: 9522
🏁 Script executed:
# Verify the exact getString call with arrayOf in the code
sed -n '468,476p' "feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt"Repository: rainxchzed/Github-Store
Length of output: 466
Downgrade detection false-positives for manually selected newer releases.
The condition checks only version string inequality (normalizeVersion(release.tagName) != normalizeVersion(installedApp.installedVersion)) combined with !isUpdateAvailable. This triggers a downgrade warning for any version mismatch where the update flag is false—including when a user manually selects a newer release from the version picker before the update check runs. Since GithubRelease has no versionCode field (extracted only after download), version ordering cannot be determined at selection time.
Compare using installedVersionCode and latestVersionCode from the InstalledApp if available, or defer the downgrade check to after version code extraction during asset installation.
Additionally, fix line 471-474: getString(Res.string.failed_to_open_app, arrayOf(installedApp.appName)) should be getString(Res.string.failed_to_open_app, installedApp.appName) to correctly pass the format argument (currently passes an Array object instead of the String).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`
around lines 304 - 322, The downgrade-warning logic in DetailsViewModel
incorrectly treats any normalized-version mismatch as a downgrade (causing false
positives when a user manually selects a newer release) — update the check in
the block that builds and sends DetailsEvent.ShowDowngradeWarning to (a) prefer
comparing numeric version codes when available (use
InstalledApp.installedVersionCode and InstalledApp.latestVersionCode if present)
and only treat as a downgrade when installedVersionCode > targetVersionCode, and
(b) if version codes are not yet available, defer emitting the downgrade warning
until after asset installation/version-code extraction rather than relying
solely on normalizeVersion(release.tagName); additionally, fix the string
formatting call that currently passes an Array by replacing
getString(Res.string.failed_to_open_app, arrayOf(installedApp.appName)) with
getString(Res.string.failed_to_open_app, installedApp.appName) so the appName is
passed as the format argument.
There was a problem hiding this comment.
@coderabbitai Is this is the fix?:
if (installedApp != null &&
!installedApp.isPendingInstall &&
!installedApp.isUpdateAvailable &&
normalizeVersion(release.tagName) != normalizeVersion(installedApp.installedVersion) &&
installedApp.installedVersionCode != installedApp.latestVersionCode &&
platform == Platform.ANDROID
) {There was a problem hiding this comment.
@rainxchzed, not quite—your fix is on the right track but has a couple of issues:
-
Comparison direction: Adding
installedApp.installedVersionCode != installedApp.latestVersionCodechecks if they're different, not if it's a downgrade. A downgrade means installing an older version, so you needinstalledApp.installedVersionCode > targetVersionCode. -
Version code mismatch: The real problem is that
installedApp.latestVersionCoderepresents the latest version from a previous update check, which might not be the same as the version the user just selected in the UI (release.tagName). SinceGithubReleasehas noversionCodefield (it's only extracted after downloading the APK), you can't know at selection time whether the chosen release is actually older. -
The
getStringbug: You also need to fix line 471-474 where it currently hasgetString(Res.string.failed_to_open_app, arrayOf(installedApp.appName))— remove thearrayOf()wrapper so it's justgetString(Res.string.failed_to_open_app, installedApp.appName).
Better approach:
if (installedApp != null &&
!installedApp.isPendingInstall &&
!installedApp.isUpdateAvailable &&
normalizeVersion(release.tagName) != normalizeVersion(installedApp.installedVersion) &&
platform == Platform.ANDROID
) {
// Only show downgrade warning if we can confirm it's actually older
val isConfirmedDowngrade = if (
normalizeVersion(release.tagName) == normalizeVersion(installedApp.latestVersion) &&
installedApp.latestVersionCode > 0
) {
// Selected release matches the tracked latest version
installedApp.installedVersionCode > installedApp.latestVersionCode
} else {
// Can't determine version ordering without version codes
// For safety, show warning for any version mismatch when not flagged as update
true
}
if (isConfirmedDowngrade) {
viewModelScope.launch {
_events.send(
DetailsEvent.ShowDowngradeWarning(
packageName = installedApp.packageName,
currentVersion = installedApp.installedVersion,
targetVersion = release.tagName
)
)
}
return
}
}And don't forget the getString fix in the OpenApp action handler!
🧠 Learnings used
Learnt from: CR
Repo: rainxchzed/Github-Store PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-14T03:39:05.005Z
Learning: Applies to **/*ViewModel.kt : Implement State Management using the pattern: ViewModel with MutableStateFlow for state, Channel for one-off events, and sealed classes for Actions
...ails/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsRoot.kt (1)
84-86: Consider narrowing the UI state type to avoid coupling it to the event.Holding
DetailsEvent.ShowDowngradeWarning?directly as Compose local state couples the presentation layer's dialog state to an event type. A small dedicated data class (or even just the two version strings) would be cleaner and survive future changes to the event shape without needing to update composable state declarations.♻️ Proposed refactor — decouple dialog state from event type
+ data class DowngradeDialogInfo( + val packageName: String, + val currentVersion: String, + val targetVersion: String, + ) + var downgradeWarning by remember { - mutableStateOf<DetailsEvent.ShowDowngradeWarning?>(null) + mutableStateOf<DowngradeDialogInfo?>(null) }Then in the event handler:
is DetailsEvent.ShowDowngradeWarning -> { - downgradeWarning = event + downgradeWarning = DowngradeDialogInfo( + packageName = event.packageName, + currentVersion = event.currentVersion, + targetVersion = event.targetVersion, + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsRoot.kt` around lines 84 - 86, The current Compose state variable downgradeWarning is typed as DetailsEvent.ShowDowngradeWarning?, coupling UI state to the event; change it to a dedicated UI model (e.g., a small data class DowngradeWarningData(val currentVersion: String, val minSupportedVersion: String) or simply Pair<String,String>), update the mutableStateOf declaration in DetailsRoot (replace mutableStateOf<DetailsEvent.ShowDowngradeWarning?> with mutableStateOf<DowngradeWarningData?>), and when handling DetailsEvent.ShowDowngradeWarning convert/map the event into DowngradeWarningData before setting downgradeWarning so the composable state is decoupled from the event type.feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt (1)
557-572: Consider an in-app confirmation before triggering uninstall
onUninstallClickis dispatched immediately on tap. The Android system dialog acts as the only safeguard. Adding an in-appAlertDialog("Uninstall<appName>?") would prevent accidental taps and align with the destructive-action UX convention; it would also allow you to surface any state-cleanup messaging before handing off to the system.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt` around lines 557 - 572, Wrap the immediate uninstall dispatch behind an in-app confirmation: replace the direct IconButton onClick={onUninstallClick} with a click handler that sets a local Boolean state (e.g., showUninstallDialog) to true; conditionally render an AlertDialog (title "Uninstall <app.name>?" or similar) when showUninstallDialog is true, with Confirm calling the existing onUninstallClick and then dismissing the dialog, and Cancel simply dismissing the dialog. Keep the existing visibility conditions (app.isPendingInstall and appItem.updateState checks) and reuse the same IconButton/Icon visual; only change the click flow to present the AlertDialog before invoking onUninstallClick.
🤖 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/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 203-213: The uninstallApp function currently calls
installer.uninstall(app.packageName) directly and uses a hardcoded error message
when sending AppsEvent.ShowError; wrap the uninstall call inside
viewModelScope.launch { ... } to match other stateful operations and preserve
structured concurrency, and replace the hardcoded string "Failed to uninstall
${app.appName}" with the localized resource (e.g.
getString(Res.string.failed_uninstall) or the project’s uninstall string key)
when constructing AppsEvent.ShowError; keep the existing logger.error call but
ensure the exception is captured inside the coroutine block so failures are
reported consistently.
---
Nitpick comments:
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt`:
- Around line 557-572: Wrap the immediate uninstall dispatch behind an in-app
confirmation: replace the direct IconButton onClick={onUninstallClick} with a
click handler that sets a local Boolean state (e.g., showUninstallDialog) to
true; conditionally render an AlertDialog (title "Uninstall <app.name>?" or
similar) when showUninstallDialog is true, with Confirm calling the existing
onUninstallClick and then dismissing the dialog, and Cancel simply dismissing
the dialog. Keep the existing visibility conditions (app.isPendingInstall and
appItem.updateState checks) and reuse the same IconButton/Icon visual; only
change the click flow to present the AlertDialog before invoking
onUninstallClick.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsRoot.kt`:
- Around line 84-86: The current Compose state variable downgradeWarning is
typed as DetailsEvent.ShowDowngradeWarning?, coupling UI state to the event;
change it to a dedicated UI model (e.g., a small data class
DowngradeWarningData(val currentVersion: String, val minSupportedVersion:
String) or simply Pair<String,String>), update the mutableStateOf declaration in
DetailsRoot (replace mutableStateOf<DetailsEvent.ShowDowngradeWarning?> with
mutableStateOf<DowngradeWarningData?>), and when handling
DetailsEvent.ShowDowngradeWarning convert/map the event into
DowngradeWarningData before setting downgradeWarning so the composable state is
decoupled from the event type.
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt
Show resolved
Hide resolved
This commit refines the downgrade detection logic and improves user feedback for various actions. The downgrade warning now uses `versionCode` for a more reliable comparison when available, preventing false positives where version names might be the same. Additionally, error handling has been improved for failed app uninstalls and launches, providing clearer messages to the user. - **refactor(details)**: Improved downgrade detection by comparing `versionCode` in `DetailsViewModel` for greater accuracy. - **fix(details)**: Added a `try-catch` block for app launch failures and now shows a localized error message. - **fix(apps)**: Wrapped the uninstall logic in `AppsViewModel` in a `try-catch` block to handle exceptions and show a localized error message. - **feat(i18n)**: Added a new string resource for failed uninstalls. - **refactor(core)**: Made the `normalizeVersion` function in `DetailsViewModel` null-safe.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt (1)
448-450:normalizeVersionis duplicated across three files — extract to a shared utility.The same logic exists in:
SmartInstallButton.kt(line 448, non-nullable)DetailsViewModel.kt(line 968, nullable with?: ""fallback)InstalledAppsRepositoryImpl.kt(line 224, non-nullable)Moving it to a shared
coreutility (e.g.,VersionUtils.kt) would centralise the normalisation rule and make future changes (e.g., handling build metadata or RC suffixes) apply consistently everywhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt` around lines 448 - 450, Extract the duplicated normalizeVersion logic into a single shared utility (e.g., create VersionUtils.kt with a public function normalizeVersion(version: String): String that does version.removePrefix("v").removePrefix("V").trim()), then replace the local implementations in SmartInstallButton (normalizeVersion), InstalledAppsRepositoryImpl (normalizeVersion) and the nullable usage in DetailsViewModel (adjust the call to pass version ?: "" or change the utility to accept String? and handle null) to call the shared VersionUtils.normalizeVersion, and remove the duplicate functions from those files so all normalization logic is centralized.
🤖 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/AndroidInstaller.kt`:
- Around line 185-192: The uninstall method in AndroidInstaller unguards
context.startActivity for the ACTION_DELETE intent, which can throw
SecurityException for device-admin or cross-profile apps; wrap the body of
uninstall (the Intent creation and context.startActivity call for
Intent.ACTION_DELETE) in a try-catch that catches SecurityException (and
optionally Exception), log the failure via Logger.e or Logger.d with the
packageName and exception, and avoid letting the exception propagate so behavior
matches other methods like openApp.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Line 212: The error arises because getString is called with
arrayOf(app.appName) so the vararg receives an Array and its toString() is used;
change the call in AppsViewModel (where
getString(Res.string.failed_to_uninstall, arrayOf(app.appName)) is used) to pass
app.appName directly as the vararg (e.g.,
getString(Res.string.failed_to_uninstall, app.appName)), mirroring the approach
used in DetailsViewModel to ensure the substitution receives the raw app name
string.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 468-472: The UninstallApp handler in DetailsViewModel calls
installer.uninstall(...) directly on the caller thread (see
DetailsAction.UninstallApp and _state.value.installedApp) so any thrown
exception (e.g., SecurityException) will crash the UI; change it to perform the
uninstall inside a coroutine and guard it with try/catch — e.g., use
viewModelScope.launch { try { installer.uninstall(installedApp.packageName) }
catch (e: Exception) { logger.error("Failed to uninstall
${installedApp.packageName}", e) } } so the call is asynchronous and exceptions
are logged/handled like in AppsViewModel.uninstallApp.
---
Duplicate comments:
In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidInstaller.kt`:
- Around line 194-210: openApp now guards getLaunchIntentForPackage with a null
check and wraps launchIntent.addFlags and context.startActivity in a try-catch
for ActivityNotFoundException while emitting Logger.w on failure; no functional
change required—leave the implementation of openApp as-is, ensuring the try
block contains launchIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) and
context.startActivity(launchIntent), and both failure branches (catching
ActivityNotFoundException and the null else) continue to call Logger.w with the
appropriate messages.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 191-193: No change required: the AppsAction.OnUninstallApp branch
correctly calls uninstallApp(action.app) within the viewModelScope.launch
wrapper and uses the localized Res.string key as addressed; leave the existing
AppsAction.OnUninstallApp handling and the uninstallApp coroutine wrapper as-is
in AppsViewModel.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`:
- Around line 129-167: The Open card currently attaches the click via
Modifier.clickable (see the ElevatedCard block using .clickable {
onAction(DetailsAction.OpenApp) }), which skips M3 accessibility semantics;
replace the Modifier.clickable usage by using ElevatedCard(onClick = {
onAction(DetailsAction.OpenApp) }, ...) and remove the .clickable from the
modifier so the ElevatedCard's onClick provides Role.Button semantics and proper
enabled handling just like the Uninstall card does.
- Around line 182-185: In the SmartInstallButton isUpdateAvailable branch,
installedApp.latestVersion may be null so calling .toString() can render the
literal "null"; update that expression to guard against null (e.g. use a
null-coalescing fallback or format only when non-null) so the stringResource
call receives a sensible fallback value instead of "null"—locate the
isUpdateAvailable branch in SmartInstallButton.kt and replace
installedApp.latestVersion.toString() with a null-safe expression (referencing
installedApp.latestVersion) that supplies an appropriate fallback or alternate
string resource.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 474-489: No changes required: the DetailsAction.OpenApp handler in
DetailsViewModel correctly retrieves installedApp, calls
installer.openApp(installedApp.packageName), and dispatches a
DetailsEvent.OnMessage via viewModelScope.launch using
getString(Res.string.failed_to_open_app, installedApp.appName) without the
obsolete arrayOf() wrapper; leave the OpenApp branch as-is.
- Around line 298-341: The downgrade detection logic in DetailsViewModel for
DetailsAction.InstallPrimary (specifically the isConfirmedDowngrade calculation
using normalizeVersion and installedVersionCode/latestVersionCode) correctly
implements the intended behavior, so no code changes are required; keep the
existing checks in the InstallPrimary branch and proceed with the existing flow
that sends DetailsEvent.ShowDowngradeWarning when isConfirmedDowngrade is true
and otherwise calls installAsset(downloadUrl = primary.downloadUrl, assetName =
primary.name, sizeBytes = primary.size, releaseTag = release.tagName).
---
Nitpick comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`:
- Around line 448-450: Extract the duplicated normalizeVersion logic into a
single shared utility (e.g., create VersionUtils.kt with a public function
normalizeVersion(version: String): String that does
version.removePrefix("v").removePrefix("V").trim()), then replace the local
implementations in SmartInstallButton (normalizeVersion),
InstalledAppsRepositoryImpl (normalizeVersion) and the nullable usage in
DetailsViewModel (adjust the call to pass version ?: "" or change the utility to
accept String? and handle null) to call the shared
VersionUtils.normalizeVersion, and remove the duplicate functions from those
files so all normalization logic is centralized.
core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidInstaller.kt
Show resolved
Hide resolved
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt
Outdated
Show resolved
Hide resolved
...ails/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt
Show resolved
Hide resolved
This commit adds the `REQUEST_DELETE_PACKAGES` permission to the Android manifest, enabling the app to request uninstallation of other packages. It also improves error handling and robustness for app installation and uninstallation flows. - **feat(android)**: Added the `REQUEST_DELETE_PACKAGES` permission to `AndroidManifest.xml`. - **fix(details)**: Wrapped the uninstall action in `DetailsViewModel` within a `try-catch` block to gracefully handle failures and show a user-facing error message. - **fix(installer)**: Added a `try-catch` block in `AndroidInstaller` when starting the uninstall activity to prevent crashes. - **refactor(apps)**: Corrected the string formatting calls in `AppsViewModel` to properly pass arguments for error messages.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt (1)
460-467:⚠️ Potential issue | 🟡 Minor
arrayOf(e.message)was not fixed here — same bug that was corrected on lines 230, 243, and 395.The PR explicitly fixed
arrayOf(appName)→appNamein three places, but thisarrayOf(e.message)in theupdateAllAppsfailure path was missed.getStringreceives theArray<String?>object and.toString()yields[Ljava.lang.String;@...in the rendered snackbar instead of the actual error message.🐛 Proposed fix
_events.send( AppsEvent.ShowError( getString( Res.string.update_all_failed, - arrayOf(e.message) + e.message ?: "" ) ) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt` around lines 460 - 467, The error message is wrapped in an array for the failure path in AppsViewModel.updateAllApps, causing getString to receive an Array and render “[Ljava.lang.String;@...”; locate the _events.send call that constructs AppsEvent.ShowError (the getString(Res.string.update_all_failed, arrayOf(e.message)) call) and pass the error string itself instead of an array (e.g. use e.message or e.message ?: "" as the second argument to getString) so the snackbar shows the actual error text.
🧹 Nitpick comments (5)
core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidInstaller.kt (1)
60-98:when(systemArch)branches are all structurally identical — simplify to a single expression.Each
whenbranch evaluates toif (AssetArchitectureMatcher.isExactMatch(name, <same-arch-as-branch>)) 10000 else 0. Since the branch variable issystemArch, the whole block reduces to one line.♻️ Proposed simplification
- return assetsToConsider.maxByOrNull { asset -> - val name = asset.name.lowercase() - val archBoost = when (systemArch) { - SystemArchitecture.X86_64 -> { - if (AssetArchitectureMatcher.isExactMatch( - name, - SystemArchitecture.X86_64 - ) - ) 10000 else 0 - } - - SystemArchitecture.AARCH64 -> { - if (AssetArchitectureMatcher.isExactMatch( - name, - SystemArchitecture.AARCH64 - ) - ) 10000 else 0 - } - - SystemArchitecture.X86 -> { - if (AssetArchitectureMatcher.isExactMatch( - name, - SystemArchitecture.X86 - ) - ) 10000 else 0 - } - - SystemArchitecture.ARM -> { - if (AssetArchitectureMatcher.isExactMatch( - name, - SystemArchitecture.ARM - ) - ) 10000 else 0 - } - - SystemArchitecture.UNKNOWN -> 0 - } - archBoost + asset.size - } + return assetsToConsider.maxByOrNull { asset -> + val name = asset.name.lowercase() + val archBoost = if (systemArch != SystemArchitecture.UNKNOWN && + AssetArchitectureMatcher.isExactMatch(name, systemArch) + ) 10000 else 0 + archBoost + asset.size + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidInstaller.kt` around lines 60 - 98, The when(systemArch) block inside the assetsToConsider.maxByOrNull comparator is repetitive; replace the multiple SystemArchitecture branches with a single expression that calls AssetArchitectureMatcher.isExactMatch(name, systemArch) and yields 10000 or 0 accordingly (so archBoost = if (AssetArchitectureMatcher.isExactMatch(name, systemArch)) 10000 else 0), preserving the existing archBoost + asset.size calculation used by maxByOrNull.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt (3)
980-982:normalizeVersionis duplicated across three files — extract to a shared utility.Identical logic exists in
SmartInstallButton.kt(line 447, non-nullable) andInstalledAppsRepositoryImpl.kt(line 224, non-nullable). This is the third copy, differing only in acceptingString?. This should live in a shared core module (e.g.,core/domain/.../utils/VersionUtils.kt) with the nullable variant as the canonical implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 980 - 982, Extract the duplicated normalizeVersion logic into a shared utility (e.g., create VersionUtils with a nullable canonical function normalizeVersion(version: String?): String) and replace the three local implementations (normalizeVersion in DetailsViewModel, the version helper in SmartInstallButton, and in InstalledAppsRepositoryImpl) to call that shared function; keep the canonical function nullable-aware (accept String?) and return "" for null/empty, update callers in SmartInstallButton and InstalledAppsRepositoryImpl to use the shared version (removing their local non-null overloads) and update imports/references accordingly so all three files delegate to VersionUtils.normalizeVersion.
310-318:else { true }produces false-positive downgrade warnings when a user manually selects any non-tracked newer release.The
elsebranch fires whenevernormalizeVersion(release.tagName) != normalizeVersion(installedApp.latestVersion)— which is the common case when a user browses the version picker and selects a release that was published after the currently trackedlatestVersion. In that scenarioisConfirmedDowngrade = trueand the user sees a "downgrade" warning for a version that is actually newer. The conservative intent is clear (fail-safe), but the UX consequence is that the warning fires for the most typical version-picker interaction.A safer heuristic: only treat it as a confirmed downgrade when you have sufficient evidence (version codes). When evidence is absent, skip the warning rather than producing a false alarm.
♻️ Proposed improvement
- val isConfirmedDowngrade = if ( - normalizeVersion(release.tagName) == normalizeVersion(installedApp.latestVersion) && - (installedApp.latestVersionCode ?: 0L) > 0 - ) { - installedApp.installedVersionCode > (installedApp.latestVersionCode - ?: 0L) - } else { - true - } + val isConfirmedDowngrade = when { + // Same tag as tracked latest + version codes available: use them + normalizeVersion(release.tagName) == normalizeVersion(installedApp.latestVersion) && + (installedApp.latestVersionCode ?: 0L) > 0 -> + installedApp.installedVersionCode > (installedApp.latestVersionCode ?: 0L) + // Version codes unavailable AND tag differs from both installed and latest: + // we cannot confirm ordering — don't show a false-positive warning + else -> false + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 310 - 318, The current isConfirmedDowngrade computation (involving normalizeVersion(release.tagName) and installedApp.latestVersion) sets else { true } which causes false-positive downgrade warnings for releases that simply don't match the tracked latestVersion; change the logic so a downgrade is considered true only when you have explicit version-code evidence (compare installedApp.installedVersionCode > installedApp.latestVersionCode when both are non-null and positive) and otherwise default to false (i.e., do not warn if latestVersionCode is missing or comparison cannot be made); update the isConfirmedDowngrade calculation around normalizeVersion(...), installedApp.installedVersionCode and installedApp.latestVersionCode to implement this safer heuristic.
486-501:installer.openApp()is called without a try-catch — any non-ActivityNotFoundExceptionpropagates uncaught to the UI.
AndroidInstaller.openApponly catchesActivityNotFoundException; aSecurityException(cross-profile scenario) or similar would propagate throughonActionand crash the app. TheAppsViewModel.openAppuses a fulltry { … } catch (e: Exception) { … }wrapper for the same call chain.🛡️ Proposed fix
DetailsAction.OpenApp -> { val installedApp = _state.value.installedApp ?: return - val launched = installer.openApp(installedApp.packageName) - if (!launched) { - viewModelScope.launch { - _events.send( - DetailsEvent.OnMessage( - getString( - Res.string.failed_to_open_app, - installedApp.appName - ) - ) - ) - } + viewModelScope.launch { + try { + val launched = installer.openApp(installedApp.packageName) + if (!launched) { + _events.send( + DetailsEvent.OnMessage( + getString(Res.string.failed_to_open_app, installedApp.appName) + ) + ) + } + } catch (e: Exception) { + logger.error("Failed to open ${installedApp.packageName}: ${e.message}") + _events.send( + DetailsEvent.OnMessage( + getString(Res.string.failed_to_open_app, installedApp.appName) + ) + ) + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 486 - 501, DetailsAction.OpenApp currently calls installer.openApp(installedApp.packageName) without catching exceptions; wrap that call in a try/catch around installer.openApp in the DetailsAction.OpenApp branch (inside DetailsViewModel.onAction) to catch Exception (or at least SecurityException) and, on catch, send the same failure event via viewModelScope.launch { _events.send(DetailsEvent.OnMessage(getString(Res.string.failed_to_open_app, installedApp.appName))) } so failures (not just ActivityNotFoundException) are reported instead of crashing the UI.composeApp/src/androidMain/AndroidManifest.xml (1)
76-76:android:autoVerify="true"only takes effect when a valid Digital Asset Links file is published at the target domain.Without
https://github-store.org/.well-known/assetlinks.jsonpointing to this app's signing certificate, Android's App Links verification silently fails and the intent-filter falls back to showing the disambiguation chooser — identical to the behavior withoutautoVerify. Ensure the.well-known/assetlinks.jsonfile is in place before shipping, otherwise this attribute has no effect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/androidMain/AndroidManifest.xml` at line 76, The intent-filter has android:autoVerify="true" but App Links will only be verified if a valid Digital Asset Links file exists on the domain; publish a correct https://github-store.org/.well-known/assetlinks.json that includes this app's package name and signing-certificate SHA256 (matching the release keystore) so the android:autoVerify on the intent-filter succeeds, then validate with the Digital Asset Links tester (or remove/set android:autoVerify="false" from the intent-filter until the assetlinks.json is deployed and verified).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composeApp/src/androidMain/AndroidManifest.xml`:
- Around line 10-12: The manifest currently requests QUERY_ALL_PACKAGES which
triggers Google Play’s Permissions Declaration requirement; either remove
QUERY_ALL_PACKAGES and refactor code to avoid
PackageManager.getInstalledPackages() (update AndroidPackageMonitor to track
installs via ACTION_PACKAGE_ADDED/ACTION_PACKAGE_REMOVED broadcasts and rely on
PackageManager.getLaunchIntentForPackage() only when opening a known package)
and add targeted <queries> entries for any specific packages or intent filters
needed, or keep QUERY_ALL_PACKAGES but ensure you complete and submit the Google
Play Permissions Declaration Form justifying its use; leave
REQUEST_DELETE_PACKAGES as-is since it is required for
Intent.ACTION_DELETE_PACKAGE.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 468-480: The snackbar shows the package name instead of the
user-facing app name; in the DetailsAction.UninstallApp handling (use _state to
get installedApp) change the getString call for Res.string.failed_to_uninstall
to pass installedApp.appName (user-friendly name) instead of
installedApp.packageName so DetailsEvent.OnMessage displays the app name; keep
the logger.debug/error using packageName if you want the technical identifier in
logs.
---
Outside diff comments:
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 460-467: The error message is wrapped in an array for the failure
path in AppsViewModel.updateAllApps, causing getString to receive an Array and
render “[Ljava.lang.String;@...”; locate the _events.send call that constructs
AppsEvent.ShowError (the getString(Res.string.update_all_failed,
arrayOf(e.message)) call) and pass the error string itself instead of an array
(e.g. use e.message or e.message ?: "" as the second argument to getString) so
the snackbar shows the actual error text.
---
Duplicate comments:
In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidInstaller.kt`:
- Around line 183-213: The startActivity guarding issues are already addressed:
ensure the uninstall method keeps the try/catch around context.startActivity (in
AndroidInstaller.uninstall) and the openApp method continues to get the launch
intent, add Intent.FLAG_ACTIVITY_NEW_TASK, call context.startActivity inside a
try block and specifically catch ActivityNotFoundException to log a warning and
return false (AndroidInstaller.openApp); no functional changes required—leave
the logging and returns as implemented to mark the issue resolved.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 191-217: No change needed: the uninstallApp function correctly
wraps installer.uninstall in viewModelScope.launch with try/catch, logs via
logger.debug/error and sends AppsEvent.ShowError using
getString(Res.string.failed_to_uninstall, app.appName); keep the current
implementation of uninstallApp as-is.
---
Nitpick comments:
In `@composeApp/src/androidMain/AndroidManifest.xml`:
- Line 76: The intent-filter has android:autoVerify="true" but App Links will
only be verified if a valid Digital Asset Links file exists on the domain;
publish a correct https://github-store.org/.well-known/assetlinks.json that
includes this app's package name and signing-certificate SHA256 (matching the
release keystore) so the android:autoVerify on the intent-filter succeeds, then
validate with the Digital Asset Links tester (or remove/set
android:autoVerify="false" from the intent-filter until the assetlinks.json is
deployed and verified).
In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidInstaller.kt`:
- Around line 60-98: The when(systemArch) block inside the
assetsToConsider.maxByOrNull comparator is repetitive; replace the multiple
SystemArchitecture branches with a single expression that calls
AssetArchitectureMatcher.isExactMatch(name, systemArch) and yields 10000 or 0
accordingly (so archBoost = if (AssetArchitectureMatcher.isExactMatch(name,
systemArch)) 10000 else 0), preserving the existing archBoost + asset.size
calculation used by maxByOrNull.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 980-982: Extract the duplicated normalizeVersion logic into a
shared utility (e.g., create VersionUtils with a nullable canonical function
normalizeVersion(version: String?): String) and replace the three local
implementations (normalizeVersion in DetailsViewModel, the version helper in
SmartInstallButton, and in InstalledAppsRepositoryImpl) to call that shared
function; keep the canonical function nullable-aware (accept String?) and return
"" for null/empty, update callers in SmartInstallButton and
InstalledAppsRepositoryImpl to use the shared version (removing their local
non-null overloads) and update imports/references accordingly so all three files
delegate to VersionUtils.normalizeVersion.
- Around line 310-318: The current isConfirmedDowngrade computation (involving
normalizeVersion(release.tagName) and installedApp.latestVersion) sets else {
true } which causes false-positive downgrade warnings for releases that simply
don't match the tracked latestVersion; change the logic so a downgrade is
considered true only when you have explicit version-code evidence (compare
installedApp.installedVersionCode > installedApp.latestVersionCode when both are
non-null and positive) and otherwise default to false (i.e., do not warn if
latestVersionCode is missing or comparison cannot be made); update the
isConfirmedDowngrade calculation around normalizeVersion(...),
installedApp.installedVersionCode and installedApp.latestVersionCode to
implement this safer heuristic.
- Around line 486-501: DetailsAction.OpenApp currently calls
installer.openApp(installedApp.packageName) without catching exceptions; wrap
that call in a try/catch around installer.openApp in the DetailsAction.OpenApp
branch (inside DetailsViewModel.onAction) to catch Exception (or at least
SecurityException) and, on catch, send the same failure event via
viewModelScope.launch {
_events.send(DetailsEvent.OnMessage(getString(Res.string.failed_to_open_app,
installedApp.appName))) } so failures (not just ActivityNotFoundException) are
reported instead of crashing the UI.
| <uses-permission android:name="android.permission.QUERY_ALL_PACKAGES" | ||
| tools:ignore="PackageVisibilityPolicy,QueryAllPackagesPermission" /> | ||
| <uses-permission android:name="android.permission.REQUEST_DELETE_PACKAGES" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check manifest for <queries> entries and package visibility approach
echo "=== Checking AndroidManifest.xml for queries and package visibility ==="
grep -n "queries\|QUERY_ALL\|intent-filter\|action\|category" composeApp/src/androidMain/AndroidManifest.xml || true
echo -e "\n=== Full manifest content to understand structure ==="
cat composeApp/src/androidMain/AndroidManifest.xmlRepository: rainxchzed/Github-Store
Length of output: 5714
🏁 Script executed:
#!/bin/bash
# Check how package-related APIs are used in Kotlin code
echo "=== Checking for getLaunchIntentForPackage usage ==="
rg -n "getLaunchIntentForPackage" --type kotlin -A 3 -B 1
echo -e "\n=== Checking for getInstalledPackages usage ==="
rg -n "getInstalledPackages" --type kotlin -A 3 -B 1
echo -e "\n=== Checking for queryIntentActivities usage ==="
rg -n "queryIntentActivities" --type kotlin -A 3 -B 1
echo -e "\n=== Broader search for 'openApp' function to understand scope ==="
rg -n "openApp\|launchPackage\|resolveActivity" --type kotlin -A 5 -B 1Repository: rainxchzed/Github-Store
Length of output: 3428
QUERY_ALL_PACKAGES requires a Google Play Permissions Declaration Form — suppressing the lint warning with tools:ignore does not satisfy Play Store policy.
QUERY_ALL_PACKAGES gives visibility into the inventory of installed apps on a device and is restricted by Google Play. Apps that fail to meet the policy requirements or do not submit the Permissions Declaration Form may be removed from Google Play. This permission may not be used if the app can operate with more targeted scoped package visibility (e.g., specific <queries> entries or intent filters for ACTION_MAIN/CATEGORY_LAUNCHER).
The codebase uses getInstalledPackages() in AndroidPackageMonitor to enumerate all installed packages and getLaunchIntentForPackage() in multiple locations for the openApp flow. While the manifest already includes a <queries> element with an intent filter for ACTION_MAIN, this is insufficient — getInstalledPackages() genuinely requires QUERY_ALL_PACKAGES to retrieve the full package list.
You must submit a Google Play Permissions Declaration Form justifying this permission, or refactor the package enumeration logic to avoid getInstalledPackages() entirely (e.g., by listening to ACTION_PACKAGE_ADDED/ACTION_PACKAGE_REMOVED broadcasts instead of polling all packages).
REQUEST_DELETE_PACKAGES on line 12 is correctly added — it is required for Intent.ACTION_DELETE_PACKAGE and is a normal-protection-level permission (automatically granted).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composeApp/src/androidMain/AndroidManifest.xml` around lines 10 - 12, The
manifest currently requests QUERY_ALL_PACKAGES which triggers Google Play’s
Permissions Declaration requirement; either remove QUERY_ALL_PACKAGES and
refactor code to avoid PackageManager.getInstalledPackages() (update
AndroidPackageMonitor to track installs via
ACTION_PACKAGE_ADDED/ACTION_PACKAGE_REMOVED broadcasts and rely on
PackageManager.getLaunchIntentForPackage() only when opening a known package)
and add targeted <queries> entries for any specific packages or intent filters
needed, or keep QUERY_ALL_PACKAGES but ensure you complete and submit the Google
Play Permissions Declaration Form justifying its use; leave
REQUEST_DELETE_PACKAGES as-is since it is required for
Intent.ACTION_DELETE_PACKAGE.
| DetailsAction.UninstallApp -> { | ||
| val installedApp = _state.value.installedApp ?: return | ||
| logger.debug("Uninstalling app: ${installedApp.packageName}") | ||
| viewModelScope.launch { | ||
| try { | ||
| installer.uninstall(installedApp.packageName) | ||
| } catch (e: Exception) { | ||
| logger.error("Failed to request uninstall for ${installedApp.packageName}: ${e.message}") | ||
| _events.send( | ||
| DetailsEvent.OnMessage( | ||
| getString(Res.string.failed_to_uninstall, installedApp.packageName) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
failed_to_uninstall error passes packageName instead of appName, showing a technical identifier to the user.
Line 478 uses installedApp.packageName as the format argument, while AppsViewModel.uninstallApp (line 212) correctly uses app.appName for the same string resource. The resulting snackbar will read something like "Failed to uninstall com.example.app" instead of the readable app name.
🐛 Proposed fix
_events.send(
DetailsEvent.OnMessage(
- getString(Res.string.failed_to_uninstall, installedApp.packageName)
+ getString(Res.string.failed_to_uninstall, installedApp.appName)
)
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`
around lines 468 - 480, The snackbar shows the package name instead of the
user-facing app name; in the DetailsAction.UninstallApp handling (use _state to
get installedApp) change the getString call for Res.string.failed_to_uninstall
to pass installedApp.appName (user-friendly name) instead of
installedApp.packageName so DetailsEvent.OnMessage displays the app name; keep
the logger.debug/error using packageName if you want the technical identifier in
logs.
This commit refactors the details screen to reactively observe changes to an application's installation status. Previously, the installed app information was fetched only once when the screen loaded. Now, it updates in real-time if the app is installed or uninstalled while the user is on the details page. - **feat(data, domain)**: Introduced `getAppByRepoIdAsFlow` in the `InstalledAppsRepository` and `InstalledAppDao` to provide a reactive stream of an app's installation status. - **refactor(details)**: The `DetailsViewModel` now subscribes to the `getAppByRepoIdAsFlow` stream to continuously update the UI with the correct installation state. - **fix(apps)**: Corrected a missing click handler on the app list item, ensuring the entire card navigates to the repository details, not just a portion of it. - **chore(android)**: Removed an obsolete `DOWNLOAD_WITHOUT_NOTIFICATION` permission and unnecessary `<queries>` tags from `AndroidManifest.xml`.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt (1)
26-27: Consider addingLIMIT 1for explicitness.The SQL query is identical to the one in the suspend
getAppByRepoId(line 23), which also returns a single nullable entity. While Room handlesFlow<Entity?>by emitting only the first matching row, addingLIMIT 1makes the single-result contract self-documenting and avoids any ambiguity shouldrepoIduniqueness ever be relaxed at the table level.♻️ Proposed change
- `@Query`("SELECT * FROM installed_apps WHERE repoId = :repoId") + `@Query`("SELECT * FROM installed_apps WHERE repoId = :repoId LIMIT 1") fun getAppByRepoIdAsFlow(repoId: Long): Flow<InstalledAppEntity?>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt` around lines 26 - 27, The SQL query in InstalledAppDao's getAppByRepoIdAsFlow should explicitly return a single row; update the `@Query` for getAppByRepoIdAsFlow to add "LIMIT 1" (e.g., SELECT * FROM installed_apps WHERE repoId = :repoId LIMIT 1) so the Flow's single-entity contract matches the suspend getAppByRepoId and remains unambiguous if table constraints change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composeApp/src/androidMain/AndroidManifest.xml`:
- Around line 10-11: The manifest currently uses the restricted permission
"android.permission.REQUEST_INSTALL_PACKAGES" with only tools:ignore
suppression; that is insufficient for Play Store — remove the lint suppression
(remove tools:ignore="RequestInstallPackagesPolicy" from the <uses-permission>
entry), ensure the permission is only present if strictly required (consider
making it non-required via android:required="false" or gating it behind runtime
logic), and before publishing submit the Google Play Permissions Declaration
Form in the Play Console with the approved use-case justification for
REQUEST_INSTALL_PACKAGES so the app will pass review.
- Line 70: The manifest sets android:autoVerify="true" on the intent-filter
which requires a valid Digital Asset Links file; either remove or set autoVerify
to false on the intent-filter to avoid broken verification, or host a correct
assetlinks.json at https://github-store.org/.well-known/assetlinks.json
containing the app's package name and SHA-256 signing-certificate fingerprint so
Android App Links can be validated; update the intent-filter
(android:autoVerify) and ensure the assetlinks.json is generated with the
signing key used for the app (match the package name and fingerprint referenced
in the manifest).
---
Duplicate comments:
In `@composeApp/src/androidMain/AndroidManifest.xml`:
- Around line 7-8: The manifest still declares
android.permission.QUERY_ALL_PACKAGES with only tools:ignore (which doesn't
satisfy Play policy) and the previous <queries> entries were removed; remove the
broad QUERY_ALL_PACKAGES permission wherever possible, restore or add targeted
<queries> entries for the specific packages/intents your app needs (refer to the
<uses-permission android:name="android.permission.QUERY_ALL_PACKAGES"> and the
previously removed <queries> block) and refactor any getInstalledPackages()
usage to use package-specific checks or Intents; if you truly need broad package
visibility, keep an explicit justification and complete the Google Play
Permissions Declaration in the Play Console instead of relying on tools:ignore.
---
Nitpick comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt`:
- Around line 26-27: The SQL query in InstalledAppDao's getAppByRepoIdAsFlow
should explicitly return a single row; update the `@Query` for
getAppByRepoIdAsFlow to add "LIMIT 1" (e.g., SELECT * FROM installed_apps WHERE
repoId = :repoId LIMIT 1) so the Flow's single-entity contract matches the
suspend getAppByRepoId and remains unambiguous if table constraints change.
| <uses-permission android:name="android.permission.REQUEST_INSTALL_PACKAGES" | ||
| tools:ignore="RequestInstallPackagesPolicy" /> |
There was a problem hiding this comment.
REQUEST_INSTALL_PACKAGES also requires a Google Play policy declaration — tools:ignore is insufficient.
REQUEST_INSTALL_PACKAGES is a restricted permission on Google Play. Similar to QUERY_ALL_PACKAGES, suppressing the lint warning with tools:ignore="RequestInstallPackagesPolicy" does not satisfy the Play Store policy requirement. Apps requesting this permission must qualify under an approved use case (e.g., acting as an app store) and must submit a Permissions Declaration Form. While this app's purpose justifies the use case, the declaration must still be submitted to avoid rejection.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composeApp/src/androidMain/AndroidManifest.xml` around lines 10 - 11, The
manifest currently uses the restricted permission
"android.permission.REQUEST_INSTALL_PACKAGES" with only tools:ignore
suppression; that is insufficient for Play Store — remove the lint suppression
(remove tools:ignore="RequestInstallPackagesPolicy" from the <uses-permission>
entry), ensure the permission is only present if strictly required (consider
making it non-required via android:required="false" or gating it behind runtime
logic), and before publishing submit the Google Play Permissions Declaration
Form in the Play Console with the approved use-case justification for
REQUEST_INSTALL_PACKAGES so the app will pass review.
|
|
||
| <!-- App website links: https://github-store.org/app/?repo={owner}/{repo} --> | ||
| <intent-filter> | ||
| <intent-filter android:autoVerify="true"> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Android App Links android:autoVerify assetlinks.json verification fallback behavior
💡 Result:
What android:autoVerify="true" does
- When your app is installed/updated, Android attempts to verify each declared HTTPS host in any
VIEW/BROWSABLE/DEFAULTintent-filter that hasandroid:autoVerify="true"by fetchinghttps://<host>/.well-known/assetlinks.jsonand checking that it authorizes your app (package + signing cert SHA-256). If verification succeeds, the link becomes a verified Android App Link (so the system can open the app directly without prompting). [1]
If assetlinks.json is missing/invalid/unreachable (verification fails)
Then App Links verification does not apply for that host, so Android falls back to normal web-link intent resolution (i.e., your intent-filter may still match, but it’s treated as unverified). The user experience depends on Android version:
- Android 12+: unverified web links generally open in the default browser instead of showing the chooser/disambiguation dialog. [2][3]
- Android 11 and lower: unverified links commonly lead to a disambiguation (“Open with”) dialog (or whichever app the user previously chose as default). [2][3]
(Independently of App Links: if multiple apps can handle an implicit intent, Android may show a chooser; if only one can, it can open directly—standard intent behavior.) [4]
Practical implications (“fallback behavior”)
- Your app can still open those URLs only if the user explicitly selects it (Android 11-) or manually enables link handling in settings (Android 12+ “Open supported links”), but it won’t be the guaranteed/default handler without successful verification. [2][3]
- Common causes of verification failure include redirects, wrong SHA-256 fingerprint (e.g., Play App Signing vs debug), wrong host, invalid JSON, or serving the file incorrectly. [1]
Sources:
[1] Android Developers — Troubleshoot App Links https://developer.android.com/training/app-links/troubleshoot
[2] Android Developers Codelab — Android App Links intro (notes Android 12+ browser handling) https://developer.android.com/codelabs/android-app-links-introduction
[3] Android Developers (Medium) — Deep Links Crash Course, Part 3 (Android 12 behavior change) https://medium.com/androiddevelopers/deep-links-crash-course-part-3-troubleshooting-your-deep-links-61329fecb93
[4] Android Developers — Intents and intent filters (chooser behavior) https://developer.android.com/guide/components/intents-filters
android:autoVerify="true" requires a validated Digital Asset Links file to work correctly.
Adding autoVerify="true" to the intent-filter enables Android App Links verification. Without a valid assetlinks.json hosted at https://github-store.org/.well-known/assetlinks.json containing the correct app SHA-256 certificate fingerprint and package name, verification fails and Android falls back to treating the link as an unverified web link. On Android 12+, this typically means the link opens in the default browser without prompting the app to handle it; on Android 11 and lower, it may show a disambiguation dialog. Either way, this degrades the intended UX of seamlessly opening the link in your app.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composeApp/src/androidMain/AndroidManifest.xml` at line 70, The manifest sets
android:autoVerify="true" on the intent-filter which requires a valid Digital
Asset Links file; either remove or set autoVerify to false on the intent-filter
to avoid broken verification, or host a correct assetlinks.json at
https://github-store.org/.well-known/assetlinks.json containing the app's
package name and SHA-256 signing-certificate fingerprint so Android App Links
can be validated; update the intent-filter (android:autoVerify) and ensure the
assetlinks.json is generated with the signing key used for the app (match the
package name and fingerprint referenced in the manifest).
Summary by CodeRabbit