android: fix stale update status in PackageEventReceiver on external …#454
android: fix stale update status in PackageEventReceiver on external …#454rainxchzed merged 9 commits intomainfrom
Conversation
…app updates - Refactor `PackageEventReceiver` to detect version changes when an app is updated or reinstalled externally (e.g., via Play Store or sideloading). - Ensure `installedVersion`, `installedVersionName`, `installedVersionCode`, and `isUpdateAvailable` are all synchronized when a version change is detected. - Recompute `isUpdateAvailable` by comparing the new system version code against the latest known version to prevent the UI from incorrectly showing available updates for versions already installed. - Add detailed logging to distinguish between actual version changes and broadcast events triggered by reinstalls of the same version.
- Introduce `VersionMath` utility to provide a single source of truth for version normalization and semver-compliant comparison. - Implement `resolveExternalInstallVerdict` to produce a local, best-effort update status after external package changes (e.g., sideloading) using a priority ladder of signals (versionCode, versionName, and release tags). - Refactor `PackageEventReceiver` to handle external installs by updating local state and triggering an authoritative async update check via `InstalledAppsRepository`. - Migrate version logic from `InstalledAppsRepositoryImpl` to `VersionMath` to ensure consistency across the app and fix incorrect "update available" flags caused by non-standard version naming. - Update Claude local settings to allow `rtk grep` bash commands.
- Introduce `GithubRelease.isEffectivelyPreRelease()` as a single source of truth for identifying pre-releases. - Implement `VersionMath.isPreReleaseTag` using regex to detect common markers (alpha, beta, rc, preview, snapshot, etc.) in tag names and release titles. - Enhance pre-release filtering to combine the GitHub API `prerelease` flag with the new tag heuristic to catch releases where maintainers forgot to set the API flag. - Add diagnostic logging in `InstalledAppsRepositoryImpl` to report mismatches between GitHub API flags and tag-based heuristics. - Update `VersionPicker`, `DetailsViewModel`, and `DetailsState` to use the improved "effectively pre-release" logic for UI badges and release filtering.
WalkthroughAdds centralized version utilities and pre-release heuristics, exposes a targeted installed-version update API, updates package-install broadcast handling to compute an immediate install/update verdict and schedule authoritative re-checks, and surfaces pre-release channel controls across details data, viewmodel, and UI. Changes
Sequence DiagramsequenceDiagram
participant PM as Android PM
participant Receiver as PackageEventReceiver
participant Repo as InstalledAppsRepository
participant Verdict as VersionVerdictResolver
participant DB as InstalledApp DAO
PM->>Receiver: broadcast external install/update
Receiver->>DB: load InstalledApp by packageName
Receiver->>Receiver: extract newVersionCode/newVersionName
Receiver->>Receiver: detect changed versionCode/name
alt Version changed
Receiver->>Verdict: resolveExternalInstallVerdict(app, newVersionName, newVersionCode)
Verdict-->>Receiver: UP_TO_DATE / UPDATE_AVAILABLE / UNKNOWN
Receiver->>DB: updateInstalledVersion(packageName, installedVersion, installedVersionName, installedVersionCode, isUpdateAvailable)
Receiver->>Repo: schedule repo.checkForUpdates(packageName) on backstop scope
Repo->>Repo: network validation & authoritative update
Repo->>DB: persist canonical state
else No change
Receiver->>DB: no-op / preserve existing flags
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.claude/settings.local.json (1)
12-13: The.claude/settings.local.jsonfile naming is misleading but not problematic.The
.local.jsonsuffix typically indicates environment-specific settings, yet this is the only settings file in the.claude/directory and is intentionally committed to version control as a shared configuration. Consider renaming it to.claude/settings.jsonto better reflect that it's a shared, not environment-specific, file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/settings.local.json around lines 12 - 13, The settings file is named settings.local.json but is a shared, committed configuration; rename settings.local.json to settings.json, update any code/CI/docs that reference settings.local.json to use settings.json, and commit the rename preserving the existing contents (the entries like "WebFetch(domain:blog.jetbrains.com)" and "Bash(rtk grep *)" can be left unchanged).core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt (1)
178-183: Nit: importInstalledAppinstead of using the FQ name in the signature.♻️ Proposed import + signature cleanup
import zed.rainxch.core.domain.repository.InstalledAppsRepository import zed.rainxch.core.domain.system.PackageMonitor +import zed.rainxch.core.domain.model.InstalledApp import zed.rainxch.core.domain.util.VersionVerdict import zed.rainxch.core.domain.util.resolveExternalInstallVerdictprivate suspend fun handleExternalInstall( packageName: String, - app: zed.rainxch.core.domain.model.InstalledApp, + app: InstalledApp, repo: InstalledAppsRepository, monitor: PackageMonitor, ) {🤖 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 178 - 183, The function handleExternalInstall currently uses the fully-qualified type zed.rainxch.core.domain.model.InstalledApp in its parameter list; add an import for InstalledApp at the top of the file and change the parameter type to InstalledApp in the handleExternalInstall signature (and update any other occurrences in this file that reference the FQCN) so the code uses the simple type name instead of the fully-qualified one.core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/ExternalInstallVerdict.kt (1)
93-107: Optional: theelse → UNKNOWNbranch incompareAndDecideis unreachable.After the emptiness guard on line 99 both inputs are non-empty, and
compareNormalized(used by bothcompareVersionsandisVersionNewer) is antisymmetric, socmp < 0⇔isVersionNewer(latest, system) == true. The third arm can't fire. Harmless, but simplifying makes the intent obvious and avoids readers wondering what edge it's guarding.♻️ Suggested simplification
val cmp = VersionMath.compareVersions(system, latest) - return when { - cmp >= 0 -> VersionVerdict.UP_TO_DATE - VersionMath.isVersionNewer(latest, system) -> VersionVerdict.UPDATE_AVAILABLE - else -> VersionVerdict.UNKNOWN - } + return if (cmp >= 0) VersionVerdict.UP_TO_DATE else VersionVerdict.UPDATE_AVAILABLE }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/ExternalInstallVerdict.kt` around lines 93 - 107, The final else branch returning VersionVerdict.UNKNOWN in compareAndDecide is unreachable because after the emptiness guard compareVersions and isVersionNewer are mutually exclusive; simplify compareAndDecide (keeping the emptiness check) to return VersionVerdict.UP_TO_DATE when VersionMath.compareVersions(system, latest) >= 0 and otherwise return VersionVerdict.UPDATE_AVAILABLE, removing the unreachable VersionVerdict.UNKNOWN arm; refer to compareAndDecide, VersionMath.compareVersions, VersionMath.isVersionNewer, and the VersionVerdict enum (UP_TO_DATE, UPDATE_AVAILABLE, UNKNOWN) when making the 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
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt`:
- Around line 178-219: The current handleExternalInstall captures the stale app
snapshot and calls repo.updateApp(app.copy(...)), which can overwrite concurrent
changes; change this to perform a field-specific update or a transactional
read-modify-write: add/use an InstalledAppsRepository method like
updateInstalledVersion(packageName: String, versionName: String, versionCode:
Long, isUpdateAvailable: Boolean) and call that from handleExternalInstall
(using the computed newIsUpdateAvailable), or wrap the read and
repo.updateApp(...) call in repo.executeInTransaction { ... } so the
installed-version fields are updated atomically without stomping other fields
(refer to resolveExternalInstallVerdict and the app parameter to locate the
logic).
---
Nitpick comments:
In @.claude/settings.local.json:
- Around line 12-13: The settings file is named settings.local.json but is a
shared, committed configuration; rename settings.local.json to settings.json,
update any code/CI/docs that reference settings.local.json to use settings.json,
and commit the rename preserving the existing contents (the entries like
"WebFetch(domain:blog.jetbrains.com)" and "Bash(rtk grep *)" can be left
unchanged).
In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt`:
- Around line 178-183: The function handleExternalInstall currently uses the
fully-qualified type zed.rainxch.core.domain.model.InstalledApp in its parameter
list; add an import for InstalledApp at the top of the file and change the
parameter type to InstalledApp in the handleExternalInstall signature (and
update any other occurrences in this file that reference the FQCN) so the code
uses the simple type name instead of the fully-qualified one.
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/ExternalInstallVerdict.kt`:
- Around line 93-107: The final else branch returning VersionVerdict.UNKNOWN in
compareAndDecide is unreachable because after the emptiness guard
compareVersions and isVersionNewer are mutually exclusive; simplify
compareAndDecide (keeping the emptiness check) to return
VersionVerdict.UP_TO_DATE when VersionMath.compareVersions(system, latest) >= 0
and otherwise return VersionVerdict.UPDATE_AVAILABLE, removing the unreachable
VersionVerdict.UNKNOWN arm; refer to compareAndDecide,
VersionMath.compareVersions, VersionMath.isVersionNewer, and the VersionVerdict
enum (UP_TO_DATE, UPDATE_AVAILABLE, UNKNOWN) when making the change.
🪄 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: c49ba7f1-395a-4bf4-96e2-a9dc5d0adaa1
📒 Files selected for processing (9)
.claude/settings.local.jsoncore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/GithubReleaseExt.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/ExternalInstallVerdict.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt
- Implement a new `releaseChannel` section for the app details screen to handle GitHub pre-release and stable versioning logic. - Add a `ChannelChip` component to toggle pre-release enrollment and provide a quick action to switch back to stable versions. - Implement a "stalled-project" warning card that displays when a stable release is significantly outdated compared to available pre-releases. - Add a "What's changed" card to display a merged changelog when a user has skipped multiple versions. - Integrate these components into the `LazyListScope` using `DetailsState` to conditionally render based on app tracking status and release availability.
- Add `ToggleIncludeBetas` and `SwitchToStable` actions to support better release channel management from the Details screen. - Implement `ReleaseInsights` logic to compute "stalled stable" warnings and merged changelogs for users skipping multiple versions. - Introduce a "Switch to stable" affordance in `DetailsState` to help users roll back from pre-release versions to the latest stable release. - Add a new `releaseChannel` UI section to the Details screen for toggling beta updates. - Update `DetailsViewModel` to re-calculate release insights immediately after an app update or channel toggle. - Support concatenated release notes in the "What's New" section to show all intermediate changes since the installed version.
- Add `ToggleIncludeBetas` and `SwitchToStable` actions to support better release channel management from the Details screen. - Implement `ReleaseInsights` logic to compute "stalled stable" warnings and merged changelogs for users skipping multiple versions. - Introduce a "Switch to stable" affordance in `DetailsState` to help users roll back from pre-release versions to the latest stable release. - Add a new `releaseChannel` UI section to the Details screen for toggling beta updates. - Update `DetailsViewModel` to re-calculate release insights immediately after an app update or channel toggle. - Support concatenated release notes in the "What's New" section to show all intermediate changes since the installed version.
- Enhance `VersionPicker` to display specific pre-release labels (e.g., "Beta", "RC", "Alpha") instead of a generic "Pre-release" badge when a marker is detected in the release tag. - Implement `preReleaseMarkerLabel` utility in `VersionMath` to parse and canonicalize common pre-release suffixes from version strings. - Update `InstallationManagerImpl` to inherit the global "include betas" preference from `TweaksRepository` when registering a newly installed app. - Add various string resources for the pre-release channel UI, including channel toggles and project health warnings for stalled stable releases. - Inject `TweaksRepository` into `InstallationManagerImpl` via the shared Koin module.
- Add `updateInstalledVersion` to `InstalledAppDao` to allow targeted updates of version-related columns and the `isUpdateAvailable` flag. - Implement `updateInstalledVersion` in `InstalledAppsRepository` and its implementation to provide a safer alternative to full-row updates. - Refactor `PackageEventReceiver` to use the new atomic update method, preventing race conditions where concurrent writes to sibling columns (like favorites or pins) could be overwritten by a stale app snapshot. - Improve data integrity during external app installations and sideloading by avoiding the "read-modify-write" pattern for the entire `InstalledApp` entity.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt (1)
110-111: Implicit ordering assumption onallReleases.
latestStableReleaseusesfirstOrNull, so it only yields the most recent stable ifallReleasesis guaranteed sorted descending bypublishedAt. That's the common GitHub-API order, but nothing in this data class enforces it, and downstream uses (stalled-stable warning, "Switch to stable vX.Y.Z") silently degrade if the VM ever hands over unsorted data.Consider either documenting the ordering contract on
allReleases, or making this resilient:♻️ Suggested resilient variant
val latestStableRelease: GithubRelease? - get() = allReleases.firstOrNull { !it.isEffectivelyPreRelease() } + get() = + allReleases + .asSequence() + .filter { !it.isEffectivelyPreRelease() } + .maxByOrNull { it.publishedAt }🤖 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/DetailsState.kt` around lines 110 - 111, latestStableRelease currently assumes allReleases is pre-sorted and uses firstOrNull, which can break if order isn't guaranteed; change its implementation in the DetailsState class to be order-independent by filtering out pre-releases (use isEffectivelyPreRelease()) and then selecting the release with the latest timestamp (e.g., use maxByOrNull on publishedAt/createdAt with a safe null fallback) so it always returns the most recently published stable release regardless of allReleases ordering.
🤖 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/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt`:
- Around line 106-120: The stable quick-action Chip is rendered based only on
showSwitchToStable/state.latestStableRelease but not whether that release
actually has an installable APK; update the render logic to also require an
installable asset before showing the ChannelChip. Concretely, in the block that
references state.latestStableRelease and showSwitchToStable, check the release's
installable asset (e.g. state.latestStableRelease?.installableApk or use an
existing helper like state.canInstall(release) if present) and only create the
ChannelChip when that check succeeds; keep the existing onClick
(DetailsAction.SwitchToStable) but ensure it cannot be triggered for
non-installable releases. This change touches the ChannelChip creation and the
use of state.latestStableRelease / state.canSwitchToStable.
- Around line 84-103: The ChannelChip's contentDescriptionText is static and
doesn't convey the current includeBetas state; update the ChannelChip usage so
contentDescriptionText uses the same conditional label as the visible label
(i.e., reuse the stringResource expression that chooses between
Res.string.channel_chip_include_betas and Res.string.channel_chip_stable_only
based on includeBetas) or build an explicit state-aware string (e.g., "Include
betas, selected"/"Stable only, not selected") so screen readers hear the current
state; modify the ChannelChip call in ReleaseChannel.kt (referencing
ChannelChip, includeBetas, contentDescriptionText, stringResource,
Res.string.channel_chip_include_betas, Res.string.channel_chip_stable_only,
onAction and DetailsAction.ToggleIncludeBetas) to pass that state-aware
description.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt`:
- Around line 113-133: The KDoc and implementation disagree on whether tracking
is required; update the canSwitchToStable getter to also require the
DetailsState.isTrackingApp flag (in addition to installedApp and
latestStableRelease) so the affordance only appears for tracked apps; keep the
existing checks (installedIsPreRelease via allReleases lookup and stable.tagName
!= app.installedVersion) but add a short-circuit return false when isTrackingApp
is false to match the documented contract.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 530-533: switchToStable() currently selects latestStableRelease
and immediately dispatches InstallPrimary even when that release has no
installable asset (primaryAsset becomes null); change it to first resolve an
installable asset for the current device before calling InstallPrimary.
Specifically, after retrieving _state.value.latestStableRelease (and/or after
dispatching DetailsAction.SelectRelease(stable)), verify that the resolved
primaryAsset (or a helper that finds an installable asset for the release) is
non-null and only then dispatch DetailsAction.InstallPrimary; if no installable
asset exists, bail out (or dispatch a no-op/fallback action) so the quick action
doesn't silently fail.
- Around line 666-676: The resolvedCategory currently falls back to STABLE even
when the selected release is a pre-release; change the fallback logic so
resolvedCategory is derived from the actual selected release instead of blindly
STABLE. Specifically, in the block that computes byPrevCategory, selected and
resolvedCategory, keep using byPrevCategory when non-null, otherwise set
resolvedCategory = PRE_RELEASE if the chosen selected release satisfies
isEffectivelyPreRelease(), or = ALL if not (i.e., no stable or
pre-release-specific match); reference byPrevCategory, selected,
resolvedCategory and isEffectivelyPreRelease() when making the change.
---
Nitpick comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt`:
- Around line 110-111: latestStableRelease currently assumes allReleases is
pre-sorted and uses firstOrNull, which can break if order isn't guaranteed;
change its implementation in the DetailsState class to be order-independent by
filtering out pre-releases (use isEffectivelyPreRelease()) and then selecting
the release with the latest timestamp (e.g., use maxByOrNull on
publishedAt/createdAt with a safe null fallback) so it always returns the most
recently published stable release regardless of allReleases ordering.
🪄 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: cc1588c2-7474-4d32-bd21-473ace3231e2
📒 Files selected for processing (11)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/GithubReleaseExt.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.ktcore/presentation/src/commonMain/composeResources/values/strings.xmlfeature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/di/SharedModule.ktfeature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/system/InstallationManagerImpl.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsAction.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsRoot.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/GithubReleaseExt.kt
- core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt
| if (showSwitchToStable) { | ||
| val stable = state.latestStableRelease | ||
| if (stable != null) { | ||
| ChannelChip( | ||
| label = | ||
| stringResource( | ||
| Res.string.action_switch_to_stable, | ||
| stable.tagName, | ||
| ), | ||
| icon = Icons.Default.Restore, | ||
| tint = MaterialTheme.colorScheme.primary, | ||
| onClick = { onAction(DetailsAction.SwitchToStable) }, | ||
| contentDescriptionText = null, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Don't render the stable quick action unless that release is actually installable.
This section keys off state.canSwitchToStable, but that state only checks release identity. If the latest stable has no installable APK for the current device, the chip still appears and the action falls through into a no-op install path. Consider gating this chip on an installable asset as well.
🤖 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/sections/ReleaseChannel.kt`
around lines 106 - 120, The stable quick-action Chip is rendered based only on
showSwitchToStable/state.latestStableRelease but not whether that release
actually has an installable APK; update the render logic to also require an
installable asset before showing the ChannelChip. Concretely, in the block that
references state.latestStableRelease and showSwitchToStable, check the release's
installable asset (e.g. state.latestStableRelease?.installableApk or use an
existing helper like state.canInstall(release) if present) and only create the
ChannelChip when that check succeeds; keep the existing onClick
(DetailsAction.SwitchToStable) but ensure it cannot be triggered for
non-installable releases. This change touches the ChannelChip creation and the
use of state.latestStableRelease / state.canSwitchToStable.
| /** | ||
| * True when the install button should expose a "switch to | ||
| * stable" rollback affordance: the user is tracking this app, | ||
| * is currently on a release that's effectively a pre-release, | ||
| * and a distinct stable release exists. The handler | ||
| * (`DetailsAction.SwitchToStable`) selects the stable release | ||
| * and invokes the normal install path. | ||
| */ | ||
| val canSwitchToStable: Boolean | ||
| get() { | ||
| val app = installedApp ?: return false | ||
| val stable = latestStableRelease ?: return false | ||
| // Is the currently-installed release a pre-release? | ||
| val installedIsPreRelease = | ||
| allReleases.firstOrNull { it.tagName == app.installedVersion } | ||
| ?.isEffectivelyPreRelease() == true | ||
| if (!installedIsPreRelease) return false | ||
| // Don't offer the button if the stable release IS the | ||
| // one the user has already (same tag string). | ||
| return stable.tagName != app.installedVersion | ||
| } |
There was a problem hiding this comment.
canSwitchToStable docstring vs. implementation disagree on the tracking precondition.
The KDoc at lines 113–120 states the affordance requires that "the user is tracking this app", but the implementation only checks installedApp != null (line 123) and never reads the existing isTrackingApp field (line 57). This means an app that's installed on device but not tracked in the store will still light up the "Switch to stable" button, which contradicts the documented contract and may be surprising given the "Switch to stable" action is presumably store-scoped.
Either drop the tracking claim from the KDoc or gate on it explicitly:
🛡️ Proposed fix — gate on `isTrackingApp`
val canSwitchToStable: Boolean
get() {
+ if (!isTrackingApp) return false
val app = installedApp ?: return false
val stable = latestStableRelease ?: return false
// Is the currently-installed release a pre-release?
val installedIsPreRelease =
allReleases.firstOrNull { it.tagName == app.installedVersion }
?.isEffectivelyPreRelease() == true
if (!installedIsPreRelease) return false
// Don't offer the button if the stable release IS the
// one the user has already (same tag string).
return stable.tagName != app.installedVersion
}Additionally, allReleases.firstOrNull { it.tagName == app.installedVersion } silently returns false when the installed tag has rolled off the fetched release window (old pre-release no longer in allReleases). That's a conservative fallback, but worth being aware of — users stuck on an ancient beta won't see the rollback affordance even though logically it still applies.
📝 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.
| /** | |
| * True when the install button should expose a "switch to | |
| * stable" rollback affordance: the user is tracking this app, | |
| * is currently on a release that's effectively a pre-release, | |
| * and a distinct stable release exists. The handler | |
| * (`DetailsAction.SwitchToStable`) selects the stable release | |
| * and invokes the normal install path. | |
| */ | |
| val canSwitchToStable: Boolean | |
| get() { | |
| val app = installedApp ?: return false | |
| val stable = latestStableRelease ?: return false | |
| // Is the currently-installed release a pre-release? | |
| val installedIsPreRelease = | |
| allReleases.firstOrNull { it.tagName == app.installedVersion } | |
| ?.isEffectivelyPreRelease() == true | |
| if (!installedIsPreRelease) return false | |
| // Don't offer the button if the stable release IS the | |
| // one the user has already (same tag string). | |
| return stable.tagName != app.installedVersion | |
| } | |
| /** | |
| * True when the install button should expose a "switch to | |
| * stable" rollback affordance: the user is tracking this app, | |
| * is currently on a release that's effectively a pre-release, | |
| * and a distinct stable release exists. The handler | |
| * (`DetailsAction.SwitchToStable`) selects the stable release | |
| * and invokes the normal install path. | |
| */ | |
| val canSwitchToStable: Boolean | |
| get() { | |
| if (!isTrackingApp) return false | |
| val app = installedApp ?: return false | |
| val stable = latestStableRelease ?: return false | |
| // Is the currently-installed release a pre-release? | |
| val installedIsPreRelease = | |
| allReleases.firstOrNull { it.tagName == app.installedVersion } | |
| ?.isEffectivelyPreRelease() == true | |
| if (!installedIsPreRelease) return false | |
| // Don't offer the button if the stable release IS the | |
| // one the user has already (same tag string). | |
| return stable.tagName != app.installedVersion | |
| } |
🤖 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/DetailsState.kt`
around lines 113 - 133, The KDoc and implementation disagree on whether tracking
is required; update the canSwitchToStable getter to also require the
DetailsState.isTrackingApp flag (in addition to installedApp and
latestStableRelease) so the affordance only appears for tracked apps; keep the
existing checks (installedIsPreRelease via allReleases lookup and stable.tagName
!= app.installedVersion) but add a short-circuit return false when isTrackingApp
is false to match the documented contract.
- Update `ChannelChip` accessibility by mirroring the visible label in `contentDescriptionText`, ensuring screen readers announce the current state ("Include betas" / "Stable only").
- Refine `latestStableRelease` logic to use the most recently published stable release instead of the first one found in the list.
- Enhance the "Switch to stable" rollback affordance to only be available if the latest stable release contains an installable asset for the current platform.
- Add defensive checks in `switchToStable` to bail early if no primary installable asset is found, preventing silent no-ops.
- Improve initial release category selection logic to automatically switch to `PRE_RELEASE` if a project only contains pre-releases, avoiding empty lists on start.
- Update `ReleaseInsights` and `DetailsState` to track and propagate `latestStableHasInstallableAsset` status.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt (1)
131-174: Stalled-warning month formatting truncates instead of rounding.
days / 30integer-divides, so a value of 59 days renders as "1 month" and 89 days as "2 months". For a user-facing warning that's framed as a duration estimate, rounding to the nearest month ((days + 15) / 30) reads more accurately. Also note the strings are likely passedIntvalues with a single placeholder — if your resource format supports plurals, prefer it for "1 month" vs "2 months" grammar across locales.🤖 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/sections/ReleaseChannel.kt` around lines 131 - 174, The code uses integer division when computing months from stalledStableSinceDays, causing truncation (e.g., 59 -> 1); in the UI block that builds title (see stalledStableSinceDays, days, title and stringResource(Res.string.stalled_project_warning_months)), change the month calculation to round to nearest month (e.g., (days + 15) / 30) before passing to stringResource; additionally, prefer using a plural resource (or locale-aware plural API) for Res.string.stalled_project_warning_months to handle singular vs plural correctly across locales.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt (2)
489-500: Swallowed exception indaysSinceIso.The catch silently drops any parse failure (returning
nullis the intended fallback, but losing the cause makes drift in release date formats invisible in production). Static analysis (detektSwallowedException) flagged this. Log at debug level so the silent fallback is at least traceable when investigating "why is the stalled-warning card not appearing".♻️ Suggested fix
return try { val published = kotlin.time.Instant.parse(isoTimestamp) val now = System.now() val diffMs = now.toEpochMilliseconds() - published.toEpochMilliseconds() if (diffMs < 0) null else (diffMs / MILLIS_PER_DAY).toInt() - } catch (e: Exception) { + } catch (e: Exception) { + logger.debug("daysSinceIso: failed to parse '$isoTimestamp': ${e.message}") null }🤖 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 489 - 500, The daysSinceIso function swallows parsing exceptions, making format failures invisible; modify daysSinceIso to catch the exception but log it at debug/trace level (including the isoTimestamp and exception message/stack) before returning null so behavior is unchanged but failures are traceable; locate the daysSinceIso function and add a debug log call (via the existing logger used in this ViewModel or create/use a private logger instance) inside the catch block, ensuring you still return null when parsing fails.
509-528:toggleIncludeBetasdoesn't guard against rapid double-taps.Each tap kicks off a new
checkForUpdatescoroutine without cancelling a previous in-flight one. Two quick toggles will result in two concurrent network round-trips whose final DB writes race; whichever finishes last wins, and that may not match the latest user-selected channel. Consider tracking the in-flight job and cancelling/replacing it (similar toaboutTranslationJob/whatsNewTranslationJobalready used elsewhere in this VM) so the on-screen result always reflects the user's most recent intent.🤖 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 509 - 528, toggleIncludeBetas launches unchecked checkForUpdates coroutines so rapid double-taps can leave a stale DB write winning; add a cancellable tracked Job (e.g., a private var checkForUpdatesJob: Job? on the ViewModel) and before launching in toggleIncludeBetas cancel and join the existing job, then assign the new coroutine to checkForUpdatesJob using viewModelScope.launch; ensure you still rethrow CancellationException and clear or replace checkForUpdatesJob on completion/failure (mirroring the pattern used by aboutTranslationJob/whatsNewTranslationJob) so only the latest toggle's network/DB work can complete.core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt (1)
145-169:updateInstalledVersiondoesn't bumplastUpdatedAt/lastCheckedAt.The atomic write deliberately scopes to the four version-related columns +
isUpdateAvailable, but it skips bothlastUpdatedAtandlastCheckedAt. After an external install/sideload, the apps row will continue to show the previous "last updated" timestamp, which is what users typically use to spot a version change in the apps list. Consider including at leastlastUpdatedAt = :timestamp(and possiblylastCheckedAt) so the row's metadata stays in sync with the real install event the receiver just processed.♻️ Suggested addition
`@Query`( """ UPDATE installed_apps SET installedVersion = :installedVersion, installedVersionName = :installedVersionName, installedVersionCode = :installedVersionCode, - isUpdateAvailable = :isUpdateAvailable + isUpdateAvailable = :isUpdateAvailable, + lastUpdatedAt = :timestamp WHERE packageName = :packageName """, ) suspend fun updateInstalledVersion( packageName: String, installedVersion: String, installedVersionName: String?, installedVersionCode: Long, isUpdateAvailable: Boolean, + timestamp: Long, )🤖 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 145 - 169, The UPDATE in updateInstalledVersion omits lastUpdatedAt (and optionally lastCheckedAt), so external installs leave stale timestamps; update updateInstalledVersion to accept a timestamp parameter (e.g., lastUpdatedAt: Long and optionally lastCheckedAt: Long?) and include "lastUpdatedAt = :lastUpdatedAt" (and "lastCheckedAt = :lastCheckedAt" if desired) in the SQL SET clause so the row metadata is updated atomically along with installedVersion/installedVersionName/installedVersionCode/isUpdateAvailable.
🤖 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/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt`:
- Around line 226-258: The ChannelChip's Surface/Row currently uses
Modifier.padding(horizontal = 12.dp, vertical = 8.dp) around a 16.dp icon + 6.dp
spacer + label which yields a touch area under the 48 dp Material minimum;
update the composable (ChannelChip / the Surface modifier) to ensure the hit
target is at least 48.dp by adding Modifier.minimumInteractiveComponentSize()
(or explicitly enforce minHeight = 48.dp) to the Surface/Row modifier, or
replace the custom chip with a Material chip composable such as
AssistChip/FilterChip to inherit correct touch target behavior.
---
Nitpick comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt`:
- Around line 145-169: The UPDATE in updateInstalledVersion omits lastUpdatedAt
(and optionally lastCheckedAt), so external installs leave stale timestamps;
update updateInstalledVersion to accept a timestamp parameter (e.g.,
lastUpdatedAt: Long and optionally lastCheckedAt: Long?) and include
"lastUpdatedAt = :lastUpdatedAt" (and "lastCheckedAt = :lastCheckedAt" if
desired) in the SQL SET clause so the row metadata is updated atomically along
with
installedVersion/installedVersionName/installedVersionCode/isUpdateAvailable.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt`:
- Around line 131-174: The code uses integer division when computing months from
stalledStableSinceDays, causing truncation (e.g., 59 -> 1); in the UI block that
builds title (see stalledStableSinceDays, days, title and
stringResource(Res.string.stalled_project_warning_months)), change the month
calculation to round to nearest month (e.g., (days + 15) / 30) before passing to
stringResource; additionally, prefer using a plural resource (or locale-aware
plural API) for Res.string.stalled_project_warning_months to handle singular vs
plural correctly across locales.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 489-500: The daysSinceIso function swallows parsing exceptions,
making format failures invisible; modify daysSinceIso to catch the exception but
log it at debug/trace level (including the isoTimestamp and exception
message/stack) before returning null so behavior is unchanged but failures are
traceable; locate the daysSinceIso function and add a debug log call (via the
existing logger used in this ViewModel or create/use a private logger instance)
inside the catch block, ensuring you still return null when parsing fails.
- Around line 509-528: toggleIncludeBetas launches unchecked checkForUpdates
coroutines so rapid double-taps can leave a stale DB write winning; add a
cancellable tracked Job (e.g., a private var checkForUpdatesJob: Job? on the
ViewModel) and before launching in toggleIncludeBetas cancel and join the
existing job, then assign the new coroutine to checkForUpdatesJob using
viewModelScope.launch; ensure you still rethrow CancellationException and clear
or replace checkForUpdatesJob on completion/failure (mirroring the pattern used
by aboutTranslationJob/whatsNewTranslationJob) so only the latest toggle's
network/DB work can complete.
🪄 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: 3603a6bb-8ba3-45d6-a392-60362bcd2653
📒 Files selected for processing (7)
core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/InstalledAppsRepository.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt
| Surface( | ||
| shape = RoundedCornerShape(20.dp), | ||
| color = MaterialTheme.colorScheme.surfaceContainerHigh, | ||
| modifier = | ||
| Modifier | ||
| .clickable(onClick = onClick) | ||
| .then( | ||
| if (contentDescriptionText != null) { | ||
| Modifier.semantics { contentDescription = contentDescriptionText } | ||
| } else { | ||
| Modifier | ||
| }, | ||
| ), | ||
| ) { | ||
| Row( | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| modifier = Modifier.padding(horizontal = 12.dp, vertical = 8.dp), | ||
| ) { | ||
| Icon( | ||
| imageVector = icon, | ||
| contentDescription = null, | ||
| tint = tint, | ||
| modifier = Modifier.size(16.dp), | ||
| ) | ||
| Spacer(Modifier.size(6.dp)) | ||
| Text( | ||
| text = label, | ||
| style = MaterialTheme.typography.labelMedium, | ||
| color = tint, | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
ChannelChip may fall below the 48 dp minimum touch target.
With padding(horizontal = 12.dp, vertical = 8.dp) around a 16 dp icon + 6 dp spacer + label, the resulting hit area is roughly 32 dp tall — below the Material 3 minimum interactive size for touch targets, which can fail accessibility audits and is awkward for users with motor impairments. Consider using Modifier.minimumInteractiveComponentSize() or a Material chip composable (AssistChip/FilterChip) which handle this automatically.
🤖 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/sections/ReleaseChannel.kt`
around lines 226 - 258, The ChannelChip's Surface/Row currently uses
Modifier.padding(horizontal = 12.dp, vertical = 8.dp) around a 16.dp icon + 6.dp
spacer + label which yields a touch area under the 48 dp Material minimum;
update the composable (ChannelChip / the Surface modifier) to ensure the hit
target is at least 48.dp by adding Modifier.minimumInteractiveComponentSize()
(or explicitly enforce minHeight = 48.dp) to the Surface/Row modifier, or
replace the custom chip with a Material chip composable such as
AssistChip/FilterChip to inherit correct touch target behavior.
…app updates
PackageEventReceiverto detect version changes when an app is updated or reinstalled externally (e.g., via Play Store or sideloading).installedVersion,installedVersionName,installedVersionCode, andisUpdateAvailableare all synchronized when a version change is detected.isUpdateAvailableby comparing the new system version code against the latest known version to prevent the UI from incorrectly showing available updates for versions already installed.Summary by CodeRabbit
New Features
Bug Fixes / Improvements