fix(versioning): wrong-version detection — strict equality for install CTA (E3 R2)#540
Merged
Conversation
Contributor
WalkthroughThis PR introduces ChangesExact Version Detection Logic
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Microsoft Presidio Analyzer (2.2.362)core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.ktMicrosoft Presidio Analyzer failed to scan this file Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt (1)
137-147: ⚡ Quick winDeduplicate prefix cleanup to avoid future drift with
normalizeVersion.Line 137 introduces cleanup logic that duplicates the normalization prefix handling already present in
normalizeVersion(Line 53–58). Centralizing this reduces behavioral drift risk.♻️ Proposed refactor
fun normalizeVersion(version: String?): String { - if (version.isNullOrBlank()) return "" - val withoutRefs = - version - .trim() - .removePrefix("refs/tags/") - .removePrefix("v") - .removePrefix("V") - .trim() + val withoutRefs = stripCommonPrefixes(version) ?: return "" val withoutBuildMetadata = withoutRefs.substringBefore('+') if (parseSemanticVersion(withoutBuildMetadata) != null) { return withoutBuildMetadata }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt` around lines 137 - 147, stripCommonPrefixes duplicates the prefix cleanup logic found in normalizeVersion; extract the shared trimming/removal of "refs/tags/", "v", and "V" (plus trim/empty check) into a single private helper (e.g., cleanVersionPrefix) and have both normalizeVersion and stripCommonPrefixes call that helper (or have stripCommonPrefixes delegate to normalizeVersion if semantics match) so prefix handling is centralized and behavior stays consistent.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt (1)
211-213: 💤 Low valueStylistic inconsistency: AppHeader delegates version normalization to
isExactSameVersion, while SmartInstallButton pre-normalizes explicitly.Both approaches are safe.
isExactSameVersionaccepts nullable parameters and handles them robustly:
- If either input is null or blank,
stripCommonPrefixes()returns null, causing the function to returnfalse- Both inputs are internally trimmed via
stripCommonPrefixes()before comparisonHowever,
SmartInstallButton(lines 96–105) explicitly pre-trims and null-guards both inputs before passing them, whileAppHeader(line 212) passesinstalledApp.installedVersionraw andrelease?.tagNameas-is. This inconsistency makes the null-handling implicit rather than explicit.Consider aligning with
SmartInstallButton's pattern for consistency and clarity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt` around lines 211 - 213, The AppHeader.kt check currently passes installedApp.installedVersion and release?.tagName directly into VersionMath.isExactSameVersion; align this with SmartInstallButton by pre-normalizing and null-guarding both inputs before calling isExactSameVersion (e.g., apply the same trimming/strip-common-prefix behavior used in SmartInstallButton to installedApp.installedVersion and release?.tagName), so call isExactSameVersion only with the normalized values to make null/blank handling explicit and consistent.
🤖 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.
Nitpick comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt`:
- Around line 137-147: stripCommonPrefixes duplicates the prefix cleanup logic
found in normalizeVersion; extract the shared trimming/removal of "refs/tags/",
"v", and "V" (plus trim/empty check) into a single private helper (e.g.,
cleanVersionPrefix) and have both normalizeVersion and stripCommonPrefixes call
that helper (or have stripCommonPrefixes delegate to normalizeVersion if
semantics match) so prefix handling is centralized and behavior stays
consistent.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt`:
- Around line 211-213: The AppHeader.kt check currently passes
installedApp.installedVersion and release?.tagName directly into
VersionMath.isExactSameVersion; align this with SmartInstallButton by
pre-normalizing and null-guarding both inputs before calling isExactSameVersion
(e.g., apply the same trimming/strip-common-prefix behavior used in
SmartInstallButton to installedApp.installedVersion and release?.tagName), so
call isExactSameVersion only with the normalized values to make null/blank
handling explicit and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e82394ea-eff3-499b-8bd7-9c585cc5bdfc
📒 Files selected for processing (16)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.ktcore/presentation/src/commonMain/composeResources/files/whatsnew/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ar/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/bn/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/es/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/fr/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/hi/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/it/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ja/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ko/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/pl/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ru/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/tr/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/17.jsonfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes E3 R2 from
roadmap/FREE_FEATURES.md. Survey #11: "部分软件识别错误，无法安装。提示已安装相同版本，但其实是不同版本" (Some apps recognized incorrectly, cannot install. Says same version installed, but actually different version.)Root cause
VersionMath.isSameVersionis semver-correct: it strips+buildmetadata before comparing because per semver spec, build metadata is ignored for ordering. The CTA gating inSmartInstallButtonand the "Installed: X" subtext inAppHeaderconsume this same comparator, so two releases that differ only in build metadata (v1.0.0+build.1vsv1.0.0+build.2) — or maintainers who abuse+buildto ship distinct artifacts under the same numeric core — render as "Open" / hide the version subtext, telling the user the app is already installed when it actually isn't.The same false positive can fire on legitimately different releases when the maintainer's tag scheme uses the dotted-digit core for an unrelated grouping (e.g. flavor / arch suffixes that the comparator currently treats as semver pre-release strings).
Fix
Add
VersionMath.isExactSameVersion(a, b)— strict literal equality after the conservative cleanup thatnormalizeVersionalready applies before the semver normalization steps: trim, striprefs/tags/, strip leadingv/V, trim again. Build metadata is preserved. Pre-release suffixes are preserved. Case-sensitive on the suffix.Replace the three UI call sites that gate user-visible "open vs install" decisions:
SmartInstallButton.kt:105—isSameVersionInstalledflag (drives "Open" vs "Install" CTA)SmartInstallButton.kt:249— "Install version X" button-label branchAppHeader.kt:212— "Installed: X" version-subtext display gateisSameVersionis left in place forDetailsState.canSwitchToStable(best-effort tag-to-release lookup, where forgiving prefix-drift matching is desirable) and for the existingisVersionNewer/compareVersionsordering paths inInstalledAppsRepositoryImplandExternalInstallVerdict.Acceptance criteria
v1.0.0+build.1vsv1.0.0+build.2— exact-same returns false → install CTA surfaces correctlyv1.0.0vs1.0.0— exact-same returns true (prefix stripped on both sides) → still treated as samev1.0.0vsV1.0.0— exact-same returns true (both V variants stripped)v1.0.0-rc1vsv1.0.0-rc1— exact-same returns truev1.0.0-rc1vsv1.0.0— exact-same returns false (suffix preserved)v1.0.0.4vsv1.0.0.4— exact-same returns true (4-segment numeric core preserved)2024.10.15vs2024.10.15— exact-same returns true (date-style version preserved)isSameVersion, which treats two empties as equal)isVersionNewer/compareVersionssemver semantics — unchangedcanSwitchToStablelookup — unchangedTest plan
v1.0.0+sha.abc)+sha.defis latest:composeApp:compileDebugKotlinAndroid✅Notes
*Test.ktfiles in any module, so adding the regression suite forcompareVersions/isExactSameVersionrequires bringing upkotlin-test+ JVM source set wiring in:core:domainfirst. Filed for a follow-up sweep when test infra lands.What's-new
whatsnew/17.jsonacross 13 locales describing the version-detection improvement.Summary by CodeRabbit
Bug Fixes
Documentation