fix(updates): false 'update available' prompt after self-update (#515)#522
Conversation
…versionCode parity to stop false update prompts
WalkthroughThe PR fixes an issue where the GitHub Store app continued reporting an available update for itself after being updated. It adds version-code parity checks, backfill logic to pin installedVersion tags, and startup-time normalization to detect and resolve stale update states. ChangesSelf-Update False-Positive Resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt (1)
374-438:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCanary becomes inert after first
checkForUpdatesdue tolatestVersionCode = nullinupdateVersionInfo, creating a re-emergence window
updateVersionInfo(line 418, pre-existing) always writeslatestVersionCode = null. This clears the field the canary depends on. The sequence:
- Pending install resolved →
latestVersionCodeandinstalledVersionCodeare equal in DB → canary fires →isUpdateAvailable = false✓updateVersionInforuns →latestVersionCodeis nownullin DB- If the process crashes before
updateInstalledVersion(the backfill at lines 428–438) completes, the tag stays stale- Next
checkForUpdates:latestCode = null→codesAlreadyMatch = false→ falls back to tag compare → stale tag →isUpdateAvailable = true→ bug re-emerges, and the backfill never fires again (becausecodesAlreadyMatchstays false)For the self-app,
normalizeSelfInstalledVersionat cold-start covers this. For non-self apps in the broken state wherePackageEventReceiverwas also missed, no equivalent recovery exists.The fix is to preserve
latestVersionCodeinupdateVersionInfowhen codes already matched, keeping the canary durable across runs:🛡️ Proposed fix — preserve
latestVersionCodewhen canary firedinstalledAppsDao.updateVersionInfo( packageName = packageName, available = isUpdateAvailable, version = matchedRelease.tagName, assetName = primaryAsset.name, assetUrl = primaryAsset.downloadUrl, assetSize = primaryAsset.size, releaseNotes = matchedRelease.description ?: "", timestamp = System.currentTimeMillis(), latestVersionName = matchedRelease.tagName, - latestVersionCode = null, + // When codes already matched we confirmed parity — keep + // the stored code so the canary stays active on the next + // sweep even if the tag backfill below didn't complete + // atomically (`#515` defence-in-depth). + latestVersionCode = if (codesAlreadyMatch) latestCode else null, latestReleasePublishedAt = matchedRelease.publishedAt, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt` around lines 374 - 438, The canary becomes inert because installedAppsDao.updateVersionInfo is always called with latestVersionCode = null, clearing the DB field that codesAlreadyMatch relies on; change the call site in InstalledAppsRepositoryImpl (where updateVersionInfo is invoked after computing codesAlreadyMatch and isUpdateAvailable) to preserve the existing latestVersionCode when codesAlreadyMatch is true (i.e., pass the current latestVersionCode from app.latestVersionCode instead of null), so the canary persists across restarts and the backfill (updateInstalledVersion) remains effective; ensure updateVersionInfo still writes null when you truly don't have a latest code.
🧹 Nitpick comments (1)
composeApp/src/androidMain/kotlin/zed/rainxch/githubstore/app/GithubStoreApp.kt (1)
220-262: 💤 Low valueOrphaned KDoc —
resolveSelfPendingInstallis now undocumentedThe original KDoc at lines 220–225 was for
resolveSelfPendingInstall. After insertingnormalizeSelfInstalledVersion(with its own KDoc at lines 226–239) between them, the old KDoc is now a dangling block comment associated withnormalizeSelfInstalledVersionrather than its intended function.resolveSelfPendingInstallat line 262 has no KDoc.✏️ Proposed fix — move the original KDoc to its intended function
- /** - * Resolves a stale `isPendingInstall` flag for the app's own - * database row. Called on every cold start when the row exists - * and still has the flag set — the typical scenario after a - * successful self-update where the broadcast path missed. - */ /** * Self-heal stale `installedVersion` tags on the self-row carried * over from before `#515` was fixed. ... */ private suspend fun normalizeSelfInstalledVersion(...) { ... } + /** + * Resolves a stale `isPendingInstall` flag for the app's own + * database row. Called on every cold start when the row exists + * and still has the flag set — the typical scenario after a + * successful self-update where the broadcast path missed. + */ private suspend fun resolveSelfPendingInstall(...) { ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composeApp/src/androidMain/kotlin/zed/rainxch/githubstore/app/GithubStoreApp.kt` around lines 220 - 262, The KDoc for resolveSelfPendingInstall got left above normalizeSelfInstalledVersion and is now orphaned; move the original KDoc block that currently sits before normalizeSelfInstalledVersion down so it directly precedes the resolveSelfPendingInstall function declaration (and keep normalizeSelfInstalledVersion's own KDoc intact), ensuring resolveSelfPendingInstall regains its documentation and there are no duplicate or dangling KDoc comments for these methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt`:
- Around line 374-438: The canary becomes inert because
installedAppsDao.updateVersionInfo is always called with latestVersionCode =
null, clearing the DB field that codesAlreadyMatch relies on; change the call
site in InstalledAppsRepositoryImpl (where updateVersionInfo is invoked after
computing codesAlreadyMatch and isUpdateAvailable) to preserve the existing
latestVersionCode when codesAlreadyMatch is true (i.e., pass the current
latestVersionCode from app.latestVersionCode instead of null), so the canary
persists across restarts and the backfill (updateInstalledVersion) remains
effective; ensure updateVersionInfo still writes null when you truly don't have
a latest code.
---
Nitpick comments:
In
`@composeApp/src/androidMain/kotlin/zed/rainxch/githubstore/app/GithubStoreApp.kt`:
- Around line 220-262: The KDoc for resolveSelfPendingInstall got left above
normalizeSelfInstalledVersion and is now orphaned; move the original KDoc block
that currently sits before normalizeSelfInstalledVersion down so it directly
precedes the resolveSelfPendingInstall function declaration (and keep
normalizeSelfInstalledVersion's own KDoc intact), ensuring
resolveSelfPendingInstall regains its documentation and there are no duplicate
or dangling KDoc comments for these methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ed0c02c-c2e8-4cdb-91bc-e529d0ef28f3
📒 Files selected for processing (16)
composeApp/src/androidMain/kotlin/zed/rainxch/githubstore/app/GithubStoreApp.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.ktcore/presentation/src/commonMain/composeResources/files/whatsnew/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ar/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/bn/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/es/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/fr/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/hi/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/it/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ja/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ko/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/pl/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ru/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/tr/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/16.json
Closes #515
Root cause
Self-update flow leaves the row's `installedVersion` (tag string like `v1.7.0`) un-touched when reconciling the post-install state. `installedVersionName` and `installedVersionCode` get refreshed from `PackageManager`, but the tag does not. `checkForUpdates` then compares freshly-fetched `matchedRelease.tagName` (`v1.8.0`) against the stale tag (`v1.7.0`) via `VersionMath.isVersionNewer` → returns true → flips `isUpdateAvailable` back on every periodic sweep.
Three code paths shared the bug:
Fix
Test plan
Summary by CodeRabbit