feat(updater): add in-app download and install for updates#3147
Open
alltechdev wants to merge 16 commits intoMetrolistGroup:mainfrom
Open
feat(updater): add in-app download and install for updates#3147alltechdev wants to merge 16 commits intoMetrolistGroup:mainfrom
alltechdev wants to merge 16 commits intoMetrolistGroup:mainfrom
Conversation
Contributor
|
Love u bro, thanks for all that nice features 👍 |
Contributor
Author
|
I want to add dhizuku as well. Great addition Edit - Added! |
Contributor
Author
|
Update card and changelog button now matches rest of entries: 
|
Contributor
Author
|
Use this build for testing. It's 13.2.1, I just bumped it back one version for testing purposes. https://github.com/alltechdev/Metrolist-fix/releases/tag/v13.2.0 |
Contributor
Author
|
Removed session installer, don't think users will make metrolist a system app. |
|
Be aware that an in-app updater violates IzzyOnDroid inclusion criteria: |
Contributor
|
@TheLastProject thank you for the info, but I think it would be a very nice feature. Maybe we can just ignore that? Or we make two versions like the version with Chromecast and without. But that's @mostafaalagamy decision. |
|
IzzyOnDroid and F-Droid should be OK with it if you make a separate flavor without in-app updater for them to use 👍 |
Contributor
Author
Ouch, did not know that. Probably nixed then. |
Member
|
i think we should have a separate izzy build, without the updater and without gms stuff |
alltechdev
force-pushed
the
feat/download-install-updates
branch
from
069b000 to
944669e
Compare
Contributor
|
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:
📝 WalkthroughWalkthroughAdds a full APK updater: download manager, multi-backend installers (Native, Root, Shizuku, Dhizuku), installer selection UI/state, manifest/proguard/build entries for privileged APIs, Hidden API bypass at startup, and supporting strings/resources and utilities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as "Updater UI\n(UpdaterSettings)"
participant DL as "ApkDownloader"
participant INST as "AppInstaller"
participant SYS as "System/Root/Shizuku/Dhizuku"
participant RC as "InstallReceiver"
rect rgba(200,200,255,0.5)
UI->>DL: request download(downloadUrl)
DL-->>UI: emit Downloading(progress) / Completed(file) / Error
end
rect rgba(200,255,200,0.5)
UI->>INST: install(apkFile, installerType)
INST->>SYS: delegate to chosen backend (Native/Root/Shizuku/Dhizuku)
SYS-->>INST: install result or RequiresUserAction
INST-->>UI: InstallResult (Success/Error/RequiresUserAction)
end
rect rgba(255,200,200,0.5)
Note over SYS,RC: PackageInstaller commits broadcast status
RC->>UI: broadcast install status (via InstallReceiver)
UI-->>UI: show toast / prompt user / update UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/kotlin/com/metrolist/music/utils/Updater.kt (1)
174-193:⚠️ Potential issue | 🟠 Major
getAllReleaseslacks error checking and can crash on GitHub API error responses.Line 178 parses the response directly as a
JSONArraywithout checking for error objects. When GitHub returns{ "message": "Rate limit exceeded" }(or similar),JSONArray(response)throws an exception. Add the same error check used ingetLatestRelease(lines 140-143) to detect and handle these responses.Suggested fix
val response = client.get("$GITHUB_API_BASE/releases?page=$page&per_page=30") { header(HttpHeaders.UserAgent, "Metrolist/${BuildConfig.VERSION_NAME}") header(HttpHeaders.Accept, "application/vnd.github+json") }.bodyAsText() - val json = JSONArray(response) + val root = org.json.JSONTokener(response).nextValue() + if (root is JSONObject && root.has("message")) { + throw Exception(root.optString("message", "GitHub API error")) + } + val json = root as? JSONArray ?: JSONArray()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/utils/Updater.kt` around lines 174 - 193, getAllReleases currently parses the GitHub response directly into a JSONArray and will crash if the API returns an error object (e.g., rate limit message); update getAllReleases to mirror the error handling used in getLatestRelease by first checking whether the response is a JSON object error (e.g., response starts with '{' or try to parse a JSONObject and check for a "message" field) and handle that case (log/throw or return empty/stop) before attempting JSONArray(response), referring to the getAllReleases function and the error-check pattern in getLatestRelease to implement the same safeguard.
🧹 Nitpick comments (3)
app/src/main/kotlin/com/metrolist/music/ui/component/ReleaseNotesCard.kt (1)
23-23: MoveMarkdownTextto a shared UI component package.
ui.componentimporting fromui.screens.settingscreates reverse coupling. ExtractMarkdownTextintoui.component(or a dedicated markdown module) and import from there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/component/ReleaseNotesCard.kt` at line 23, The import in ReleaseNotesCard.kt brings MarkdownText from ui.screens.settings, causing reverse coupling; move the MarkdownText component/class/file into the shared UI package (e.g., package com.metrolist.music.ui.component or a dedicated markdown module) and update references: relocate the MarkdownText source, change its package declaration to com.metrolist.music.ui.component (or new module package), and replace imports in ReleaseNotesCard.kt and any other files that reference it to import com.metrolist.music.ui.component.MarkdownText; ensure build/test passes after updating package paths.app/proguard-rules.pro (1)
213-233: Consider narrowing these keep rules to actual entry points.Keeping entire dependency packages (
**) is likely larger than necessary. Prefer targeted keep rules for the exact reflected/binder classes used by installer code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/proguard-rules.pro` around lines 213 - 233, The current proguard rules are too broad (e.g., -keep class rikka.shizuku.**, -keep class moe.shizuku.**, -keep class dev.rikka.tools.refine.**, -keep class com.topjohnwu.superuser.**, -keep class com.rosan.dhizuku.**) and should be narrowed to only the actual entry points used by the installer/reflection code; replace each ** rule with explicit fully-qualified classes or tight patterns for the real reflected/binder entry points (for example the Shizuku client/service class names, any specific Stub/Proxy classes, and the exact classes your installer invokes via reflection), and use -keepclassmembers or -keepnames for just the fields/methods accessed reflectively instead of keeping entire packages, while leaving the already-specific hidden Android binder rules (android.content.pm.IPackageManager, IPackageInstaller, PackageInstallerHidden, PackageManagerHidden, etc.) as-is.app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt (1)
64-71: MakegetAvailableInstallers()reflect actual capability.This always returns every backend even though the same object already exposes
hasRootAccess(),hasShizukuOrSui(), andhasDhizuku()checks. Either populateInstallerInfo.availablehere or rename the API to something likegetInstallers()so callers do not treat it as filtered availability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt` around lines 64 - 71, getAvailableInstallers currently returns every InstallerRegistry entry even when some backends are not actually usable; change it to populate InstallerInfo.available correctly instead of returning an unfiltered list. In getAvailableInstallers(), build the list from InstallerRegistry.NATIVE, ROOT, SHIZUKU, DHIZUKU but set each InstallerInfo.available based on the corresponding capability checks (use hasRootAccess() for ROOT, hasShizukuOrSui() for SHIZUKU, hasDhizuku() for DHIZUKU, and true for NATIVE), so callers receive installer objects that reflect real availability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/AndroidManifest.xml`:
- Line 11: The manifest currently declares the updater permission
android.permission.REQUEST_INSTALL_PACKAGES and updater components
InstallReceiver and ShizukuProvider unconditionally (even when
BuildConfig.UPDATER_AVAILABLE = false); move these entries out of the main
manifest into the updater flavor manifest or add tools:node="remove" overrides
in app/src/izzy/AndroidManifest.xml to strip them for the izzy build:
specifically target the <uses-permission
android:name="android.permission.REQUEST_INSTALL_PACKAGES" />, the <receiver>
with android:name=".install.InstallReceiver", and the <provider> with
android:name=".install.ShizukuProvider" in the izzy manifest and remove them so
they are not packaged when UPDATER_AVAILABLE is false.
- Line 4: The Android manifest currently sets android:testOnly="true" on the
application element which marks the APK as test-only and prevents publishing;
remove the android:testOnly="true" attribute from the
app/src/main/AndroidManifest.xml application tag (or conditionally move it into
a debug-only manifest/variant) so release builds are not blocked and the
generated APK can be installed/published.
In `@app/src/main/kotlin/com/metrolist/music/App.kt`:
- Around line 69-72: Gate the hidden API bypass call to only run when updater
support is enabled and make it safe: wrap the call to
HiddenApiBypass.addHiddenApiExemptions("I", "L") in a conditional that checks
BuildConfig.UPDATER_AVAILABLE and surround the call with try/catch to
swallow/log any Throwable so a thrown exception cannot crash App onCreate; place
this logic where the current unconditional call exists (e.g., in App.onCreate)
and use the app logger to record a warning or error if the exemption call fails.
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt`:
- Line 227: The admonition body is being flattened by using
admonitionContent.joinToString(" "), so update the rendering in
ChangelogScreen's AdmonitionBlock call to preserve original line breaks by
joining with "\n" (i.e., use admonitionContent.joinToString("\n")); target the
AdmonitionBlock invocation and the variables currentAdmonition and
admonitionContent when making this change.
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt`:
- Around line 215-230: The collection launched with coroutineScope.launch
(wrapping ApkDownloader.downloadApk(...).collect { ... }) needs to be tracked
and cancelable: store the Job returned by launch (e.g., downloadJob) and cancel
that Job from the UI "Cancel" handler so the collecting coroutine is cancelled;
update the cancel handler to also clear state (downloadState, downloadedApkFile,
downloadedBytes, totalBytes). In addition, make ApkDownloader.downloadApk
cooperative with coroutine cancellation (ensure its network/streaming code
cancels the HTTP call and deletes any partial file when the coroutine is
cancelled) so cancellation closes the connection and cleans up partial files;
finally, guard state updates from late emissions by tying emitted updates to the
active Job/session (or ignoring emissions when downloadJob is null or not equal
to the current session) so DownloadState and downloadedApkFile cannot be flipped
back by stray events. Ensure changes apply to the same pattern used at the other
occurrence mentioned (lines 560-565).
- Around line 192-202: The code is performing UI state mutations (latestVersion,
updateAvailable, changelogContent, releaseInfo, checkError) inside
withContext(Dispatchers.IO); keep the network call on IO but update Compose
state on the main thread instead: call Updater.checkForUpdate(forceRefresh =
true) inside withContext(Dispatchers.IO) to get the Result, then switch to
withContext(Dispatchers.Main) (or Dispatchers.Main.immediate) and perform the
onSuccess/onFailure assignments to latestVersion, updateAvailable,
changelogContent, releaseInfo, and checkError so all mutableStateOf updates
happen on the main dispatcher.
In `@app/src/main/kotlin/com/metrolist/music/utils/ApkDownloader.kt`:
- Around line 32-33: The downloader currently writes directly to APK_FILE_NAME
and getDownloadedApk() treats any existing file as a completed download; change
the flow to write to a temporary path (e.g., APK_FILE_NAME + ".tmp") during
download within the ApkDownloader class (affecting the download method that uses
BUFFER_SIZE) and only atomically rename/move the temp file to APK_FILE_NAME
after the full successful download and integrity checks; ensure all error,
exception, and cancellation paths delete the temp file (and do not touch
APK_FILE_NAME), and update getDownloadedApk() to consider only the final
APK_FILE_NAME as a valid completed file (ignore or remove any leftover temp
files).
In `@app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt`:
- Line 50: The constant PLAY_PACKAGE_NAME and its use as the installer identity
are incorrect for non-Play-distributed builds; replace usages of
PLAY_PACKAGE_NAME with the app's actual package name (use context.packageName)
when constructing PackageInstallerHidden and when building installer commands
that use the "-i" parameter so the installer identifies this app as the
installer (not "com.android.vending"); update the constant or remove it and
ensure all references in AppInstaller (including the PackageInstallerHidden
constructor calls and any command strings that currently use PLAY_PACKAGE_NAME)
use context.packageName consistently.
- Around line 176-219: The install flows create a PackageInstaller session but
currently return on errors without abandoning it; update installRoot to call "pm
install-abandon $sessionId" before every early return after a successful
install-create (on session ID parse failure, writeResult failure, or commit
failure) and ensure the abandon is invoked even on exceptions; likewise modify
installShizuku and installDhizuku to call session.abandon() in their catch/error
paths if the session has been created and not committed, and always call
session.close() in a finally block; reference the functions/methods installRoot,
installShizuku, installDhizuku, the local variable sessionId in root flow, and
session.sessionId/session.abandon()/session.close() in Shizuku/Dhizuku flows
when making these changes.
---
Outside diff comments:
In `@app/src/main/kotlin/com/metrolist/music/utils/Updater.kt`:
- Around line 174-193: getAllReleases currently parses the GitHub response
directly into a JSONArray and will crash if the API returns an error object
(e.g., rate limit message); update getAllReleases to mirror the error handling
used in getLatestRelease by first checking whether the response is a JSON object
error (e.g., response starts with '{' or try to parse a JSONObject and check for
a "message" field) and handle that case (log/throw or return empty/stop) before
attempting JSONArray(response), referring to the getAllReleases function and the
error-check pattern in getLatestRelease to implement the same safeguard.
---
Nitpick comments:
In `@app/proguard-rules.pro`:
- Around line 213-233: The current proguard rules are too broad (e.g., -keep
class rikka.shizuku.**, -keep class moe.shizuku.**, -keep class
dev.rikka.tools.refine.**, -keep class com.topjohnwu.superuser.**, -keep class
com.rosan.dhizuku.**) and should be narrowed to only the actual entry points
used by the installer/reflection code; replace each ** rule with explicit
fully-qualified classes or tight patterns for the real reflected/binder entry
points (for example the Shizuku client/service class names, any specific
Stub/Proxy classes, and the exact classes your installer invokes via
reflection), and use -keepclassmembers or -keepnames for just the fields/methods
accessed reflectively instead of keeping entire packages, while leaving the
already-specific hidden Android binder rules
(android.content.pm.IPackageManager, IPackageInstaller, PackageInstallerHidden,
PackageManagerHidden, etc.) as-is.
In `@app/src/main/kotlin/com/metrolist/music/ui/component/ReleaseNotesCard.kt`:
- Line 23: The import in ReleaseNotesCard.kt brings MarkdownText from
ui.screens.settings, causing reverse coupling; move the MarkdownText
component/class/file into the shared UI package (e.g., package
com.metrolist.music.ui.component or a dedicated markdown module) and update
references: relocate the MarkdownText source, change its package declaration to
com.metrolist.music.ui.component (or new module package), and replace imports in
ReleaseNotesCard.kt and any other files that reference it to import
com.metrolist.music.ui.component.MarkdownText; ensure build/test passes after
updating package paths.
In `@app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt`:
- Around line 64-71: getAvailableInstallers currently returns every
InstallerRegistry entry even when some backends are not actually usable; change
it to populate InstallerInfo.available correctly instead of returning an
unfiltered list. In getAvailableInstallers(), build the list from
InstallerRegistry.NATIVE, ROOT, SHIZUKU, DHIZUKU but set each
InstallerInfo.available based on the corresponding capability checks (use
hasRootAccess() for ROOT, hasShizukuOrSui() for SHIZUKU, hasDhizuku() for
DHIZUKU, and true for NATIVE), so callers receive installer objects that reflect
real availability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c613b819-2eaa-4fe3-b54a-a859f1754511
📒 Files selected for processing (18)
app/build.gradle.ktsapp/proguard-rules.proapp/src/main/AndroidManifest.xmlapp/src/main/kotlin/com/metrolist/music/App.ktapp/src/main/kotlin/com/metrolist/music/constants/PreferenceKeys.ktapp/src/main/kotlin/com/metrolist/music/ui/component/ReleaseNotesCard.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/AccountSettings.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/SettingsScreen.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.ktapp/src/main/kotlin/com/metrolist/music/utils/ApkDownloader.ktapp/src/main/kotlin/com/metrolist/music/utils/Updater.ktapp/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.ktapp/src/main/kotlin/com/metrolist/music/utils/updater/InstallReceiver.ktapp/src/main/kotlin/com/metrolist/music/utils/updater/Installer.ktapp/src/main/res/values/metrolist_strings.xmlbuild.gradle.ktsgradle/libs.versions.toml
Comment on lines
+32
to
+33
| private const val APK_FILE_NAME = "metrolist_update.apk" | ||
| private const val BUFFER_SIZE = 8192 |
Contributor
There was a problem hiding this comment.
Persist only verified, fully downloaded APKs.
This writes straight to one fixed filename and getDownloadedApk() treats any existing file as a valid completed download. A failed download or an older APK from a previous release is therefore restored as Completed on the next visit, so the updater can offer a corrupt or stale package as the current update. Write to a temp file, delete it on every error/cancel path, and only promote it to the final name after a successful download.
Also applies to: 61-67, 106-108, 144-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/kotlin/com/metrolist/music/utils/ApkDownloader.kt` around lines
32 - 33, The downloader currently writes directly to APK_FILE_NAME and
getDownloadedApk() treats any existing file as a completed download; change the
flow to write to a temporary path (e.g., APK_FILE_NAME + ".tmp") during download
within the ApkDownloader class (affecting the download method that uses
BUFFER_SIZE) and only atomically rename/move the temp file to APK_FILE_NAME
after the full successful download and integrity checks; ensure all error,
exception, and cancellation paths delete the temp file (and do not touch
APK_FILE_NAME), and update getDownloadedApk() to consider only the final
APK_FILE_NAME as a valid completed file (ignore or remove any leftover temp
files).
Comment on lines
+176
to
+219
| return try { | ||
| val totalSize = apkFile.length() | ||
|
|
||
| // Create install session via pm | ||
| val createResult = Shell.cmd( | ||
| "pm install-create -i $PLAY_PACKAGE_NAME --user 0 -r -S $totalSize" | ||
| ).exec() | ||
|
|
||
| if (!createResult.isSuccess) { | ||
| return InstallResult.Error(createResult.err.joinToString("\n").ifEmpty { "Failed to create install session" }) | ||
| } | ||
|
|
||
| val response = createResult.out | ||
| val sessionIdPattern = Pattern.compile("(\\d+)") | ||
| val sessionIdMatcher = sessionIdPattern.matcher(response.firstOrNull() ?: "") | ||
|
|
||
| if (!sessionIdMatcher.find()) { | ||
| return InstallResult.Error("Failed to get session ID") | ||
| } | ||
|
|
||
| val sessionId = sessionIdMatcher.group(1)?.toInt() | ||
| ?: return InstallResult.Error("Invalid session ID") | ||
|
|
||
| // Write APK to session | ||
| val writeResult = Shell.cmd( | ||
| "cat \"${apkFile.absolutePath}\" | pm install-write -S ${apkFile.length()} $sessionId \"${apkFile.name}\"" | ||
| ).exec() | ||
|
|
||
| if (!writeResult.isSuccess) { | ||
| return InstallResult.Error(writeResult.err.joinToString("\n").ifEmpty { "Failed to write APK" }) | ||
| } | ||
|
|
||
| // Commit session | ||
| val commitResult = Shell.cmd("pm install-commit $sessionId").exec() | ||
|
|
||
| if (commitResult.isSuccess) { | ||
| InstallResult.Success | ||
| } else { | ||
| InstallResult.Error(commitResult.err.joinToString("\n").ifEmpty { "Install commit failed" }) | ||
| } | ||
| } catch (e: Exception) { | ||
| Log.e(TAG, "Root install failed", e) | ||
| InstallResult.Error(e.message ?: "Root install failed") | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt | sed -n '170,225p'Repository: MetrolistGroup/Metrolist
Length of output: 2622
🏁 Script executed:
cat -n app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt | sed -n '226,310p'Repository: MetrolistGroup/Metrolist
Length of output: 4386
🏁 Script executed:
cat -n app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt | sed -n '309,402p'Repository: MetrolistGroup/Metrolist
Length of output: 4744
Abandon failed install sessions before returning errors.
After a successful install-create / createSession(), the code has multiple failure paths that return errors without cleaning up the session:
-
installRoot (lines 176-219): Session ID parsing failures (lines 192-194, 197) and write/commit failures (lines 204-206, 214) return without calling
pm install-abandon $sessionId -
installShizuku (lines 235-306): APK write failures (lines 273-278), intent creation failures (lines 281-292), and commit failures (line 295) exit via exception without calling
session.abandon() -
installDhizuku (lines 324-397): Same pattern—APK write, intent creation, and commit failures don't abandon the session
Stale sessions accumulate in PackageInstaller and eventually make later update attempts fail until they're cleared. Use try-catch-finally to ensure cleanup:
- Root flow: Call
pm install-abandon $sessionIdbefore returning oninstall-writeorinstall-commiterrors - Shizuku/Dhizuku flows: Call
session.abandon()in a catch block if not yet committed, withsession.close()in finally
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt` around
lines 176 - 219, The install flows create a PackageInstaller session but
currently return on errors without abandoning it; update installRoot to call "pm
install-abandon $sessionId" before every early return after a successful
install-create (on session ID parse failure, writeResult failure, or commit
failure) and ensure the abandon is invoked even on exceptions; likewise modify
installShizuku and installDhizuku to call session.abandon() in their catch/error
paths if the session has been created and not committed, and always call
session.close() in a finally block; reference the functions/methods installRoot,
installShizuku, installDhizuku, the local variable sessionId in root flow, and
session.sessionId/session.abandon()/session.close() in Shizuku/Dhizuku flows
when making these changes.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/izzy/AndroidManifest.xml (1)
3-4: Clarify the purpose oftools:remove="android:testOnly".This attribute removal appears unrelated to the updater feature being excluded from the Izzy flavor. If the base manifest sets
android:testOnly="true", removing it here allows distribution on user devices, but this change isn't documented in the PR or the inline comment.Consider either:
- Adding a comment explaining why this is needed for the Izzy build, or
- Moving this to a separate commit if it's addressing a different issue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/izzy/AndroidManifest.xml` around lines 3 - 4, Add a brief inline comment next to the tools:remove="android:testOnly" attribute in AndroidManifest.xml explaining why the Izzy flavor removes android:testOnly (e.g., base manifest set testOnly="true" and Izzy must be distributable), or separate this change into its own commit if it addresses a different concern than the updater exclusion; reference the exact attribute tools:remove="android:testOnly" and the Izzy flavor so reviewers can see the intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/izzy/AndroidManifest.xml`:
- Around line 3-4: Add a brief inline comment next to the
tools:remove="android:testOnly" attribute in AndroidManifest.xml explaining why
the Izzy flavor removes android:testOnly (e.g., base manifest set
testOnly="true" and Izzy must be distributable), or separate this change into
its own commit if it addresses a different concern than the updater exclusion;
reference the exact attribute tools:remove="android:testOnly" and the Izzy
flavor so reviewers can see the intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 64f25f3c-eb4d-45e1-9533-852190b7b8cc
📒 Files selected for processing (1)
app/src/izzy/AndroidManifest.xml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt (2)
388-390: Make the admonition icon decorative.The adjacent label already announces the type, so TalkBack will read the same word twice here.
contentDescription = nullis enough.Minimal tweak
Icon( painter = painterResource(icon), - contentDescription = type, + contentDescription = null, tint = contentColor, modifier = Modifier.size(20.dp) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt` around lines 388 - 390, Replace the Icon's contentDescription currently set to the variable type with a null value so the icon is treated as decorative: in ChangelogScreen.kt locate the Icon composable that calls painter = painterResource(icon) and change contentDescription = type to contentDescription = null; keep the painterResource(icon) and other parameters unchanged.
232-238: Trim the narration comments in this block.
Skip empty linesandHeadersmostly restate the next branch. One higher-level note about the parser flow would keep this section easier to scan.As per coding guidelines, "Comments should be added only for complex logic or non-obvious code. Avoid redundant comments that simply restate what the code does."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt` around lines 232 - 238, Remove the redundant inline comments "Skip empty lines" and "Headers" in the parsing loop of ChangelogScreen (the block that checks trimmedLine.isBlank() and the subsequent header handling) and replace them with a single concise higher-level comment describing the parser flow (e.g., "iterate lines, skip blanks, handle headers and content accordingly") so the logic remains clear without restating code; target the loop that references trimmedLine and the header handling branch within the ChangelogScreen parsing routine.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt`:
- Line 227: AdmonitionBlock is rendering admonitionContent with plain Text which
bypasses the app's markdown rendering (so inline bold, links, and `@mentions`
aren't parsed); switch AdmonitionBlock to render its content using the same
markdown renderer/composable used elsewhere in ChangelogScreen (e.g., the
existing Markdown/RenderMarkdown composable or the markdown parsing pipeline)
instead of plain Text, passing admonitionContent.joinToString("\n") through that
renderer so links, bold, and mention click handlers behave identically (apply
the same style and click callbacks as used for the main changelog rendering in
ChangelogScreen).
---
Nitpick comments:
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt`:
- Around line 388-390: Replace the Icon's contentDescription currently set to
the variable type with a null value so the icon is treated as decorative: in
ChangelogScreen.kt locate the Icon composable that calls painter =
painterResource(icon) and change contentDescription = type to contentDescription
= null; keep the painterResource(icon) and other parameters unchanged.
- Around line 232-238: Remove the redundant inline comments "Skip empty lines"
and "Headers" in the parsing loop of ChangelogScreen (the block that checks
trimmedLine.isBlank() and the subsequent header handling) and replace them with
a single concise higher-level comment describing the parser flow (e.g., "iterate
lines, skip blanks, handle headers and content accordingly") so the logic
remains clear without restating code; target the loop that references
trimmedLine and the header handling branch within the ChangelogScreen parsing
routine.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea893744-0564-40d4-b82d-212080a773a3
📒 Files selected for processing (1)
app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt (1)
216-231:⚠️ Potential issue | 🟠 MajorWire the cancel action to the running download job.
startDownload()drops the launchedJob, so the handler on Lines 561-565 only clears local state. The flow can keep downloading/emitting afterward, which can repopulatedownloadState/downloadedApkFileafter the user already cancelled. Track the active job here, cancel it before clearing state, and make sureApkDownloader.downloadApk()stops the underlying request on cancellation too.Suggested fix
+import kotlinx.coroutines.Job ... var installError by remember { mutableStateOf<String?>(null) } var showInstallerDialog by remember { mutableStateOf(false) } + var downloadJob by remember { mutableStateOf<Job?>(null) } ... - coroutineScope.launch { + downloadJob?.cancel() + downloadJob = coroutineScope.launch { ApkDownloader.downloadApk(context, downloadUrl).collect { state -> downloadState = state when (state) { is DownloadState.Downloading -> { downloadProgress = state.progress downloadedBytes = state.downloadedBytes totalBytes = state.totalBytes } is DownloadState.Completed -> { downloadedApkFile = state.file } else -> {} } } } ... onCancelClick = { + downloadJob?.cancel() + downloadJob = null ApkDownloader.clearDownloadedApk(context) downloadState = DownloadState.Idle downloadedApkFile = null + downloadProgress = 0f + downloadedBytes = 0L + totalBytes = 0L installError = null }Also applies to: 561-565
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt` around lines 216 - 231, The cancel handler is only clearing UI state while the launched Job from startDownload() (the coroutineScope.launch that collects ApkDownloader.downloadApk()) continues running; store that Job in a variable (e.g., activeDownloadJob) when launching, and in the cancel action first call activeDownloadJob?.cancel() and await/handle its completion before resetting downloadState/downloadedApkFile/downloadProgress, and set activeDownloadJob = null; also verify/modify ApkDownloader.downloadApk() implementation to be cooperative with coroutine cancellation (propagate cancellation, close the underlying HTTP call/stream on cancellation) so the download actually stops when the Job is cancelled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt`:
- Around line 147-152: The UI currently only shows the update card when
updateAvailable && latestVersion != null, so recovered downloads set via
ApkDownloader.getDownloadedApk(context) (and downloadState =
DownloadState.Completed(file)) can be unreachable; change the card visibility to
also render when downloadState is DownloadState.Completed or downloadedApkFile
!= null (or use a new boolean like hasRecoveredApk). On restore (where
ApkDownloader.getDownloadedApk is used) set a separate displayed-version field
instead of leaving latestVersion as "" — either persist the version string at
download completion or derive it from the APK (use
PackageManager.getPackageArchiveInfo / PackageInfo) and assign it to
latestVersionDisplayed (or similar) so the card shows a meaningful version and
the install action can use downloadedApkFile and DownloadState.Completed.
- Around line 380-384: The installer picker rows use Modifier.clickable with
passive RadioButton(onClick = null), which lacks proper radio-group
accessibility; update the container Row that holds the installer options to use
Modifier.selectableGroup(), and for each option row replace Modifier.clickable
with Modifier.selectable(selected = /*bool*/, onClick = { /*toggle*/ }, role =
Role.RadioButton) while keeping the RadioButton as a visual indicator
(RadioButton(selected = /*bool*/, onClick = null)). Apply the same change to the
other radio-group pattern in this file so screen readers and keyboard users get
correct group/option semantics.
---
Duplicate comments:
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt`:
- Around line 216-231: The cancel handler is only clearing UI state while the
launched Job from startDownload() (the coroutineScope.launch that collects
ApkDownloader.downloadApk()) continues running; store that Job in a variable
(e.g., activeDownloadJob) when launching, and in the cancel action first call
activeDownloadJob?.cancel() and await/handle its completion before resetting
downloadState/downloadedApkFile/downloadProgress, and set activeDownloadJob =
null; also verify/modify ApkDownloader.downloadApk() implementation to be
cooperative with coroutine cancellation (propagate cancellation, close the
underlying HTTP call/stream on cancellation) so the download actually stops when
the Job is cancelled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eaef23ba-1d7b-47e3-aacb-d42dd322b393
📒 Files selected for processing (1)
app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt
Comment on lines
+147
to
+152
| // Check for existing downloaded APK and auto-check if cached update exists | ||
| LaunchedEffect(Unit) { | ||
| ApkDownloader.getDownloadedApk(context)?.let { file -> | ||
| downloadedApkFile = file | ||
| downloadState = DownloadState.Completed(file) | ||
| } |
Contributor
There was a problem hiding this comment.
Recovered downloads can become unreachable.
The startup path restores downloadState = Completed(file), but the card only renders when updateAvailable && latestVersion != null. If the APK is still on disk and the release metadata is missing, the user has no way to install the recovered file from this screen. Make the card reachable from DownloadState.Completed/downloadedApkFile != null as well, and persist or derive the displayed version separately instead of falling back to "".
Also applies to: 539-548
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt`
around lines 147 - 152, The UI currently only shows the update card when
updateAvailable && latestVersion != null, so recovered downloads set via
ApkDownloader.getDownloadedApk(context) (and downloadState =
DownloadState.Completed(file)) can be unreachable; change the card visibility to
also render when downloadState is DownloadState.Completed or downloadedApkFile
!= null (or use a new boolean like hasRecoveredApk). On restore (where
ApkDownloader.getDownloadedApk is used) set a separate displayed-version field
instead of leaving latestVersion as "" — either persist the version string at
download completion or derive it from the APK (use
PackageManager.getPackageArchiveInfo / PackageInfo) and assign it to
latestVersionDisplayed (or similar) so the card shows a meaningful version and
the install action can use downloadedApkFile and DownloadState.Completed.
Contributor
Author
|
@coderabbitai help |
Contributor
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
Contributor
Author
|
@coderabbitai full review |
- Add APK download with progress tracking via Kotlin Flow - Material3 UI with animated download progress card - Request install packages permission for direct installation - Handle GitHub API errors gracefully
- Navigate to updater screen from settings and account info - Auto-populate update info when navigating from update buttons - Dismiss account sheet before navigation - Add markdown support with GitHub admonitions, bold text, links - Use MarkdownText for release notes and changelog display
… Shizuku) - Add AppInstaller with support for 4 installation methods - Add installer selection dialog in UpdaterSettings - Add libsu for root installation support - Add Shizuku API with hidden API bypass for privileged installation - Add InstallReceiver for session-based installation callbacks - Add HiddenApiBypass to enable Shizuku on Android 9+
- Add ProGuard rules for Shizuku, hidden APIs, and libsu - Add Shizuku alive check before attempting installation - Fix deprecated getParcelableExtra on API 33+ - Fix state updates running on wrong thread - Fix AnimatedContent re-animating on progress updates - Use indeterminate spinner for smoother download animation - Redesign installer dialog with dividers
- Add Dhizuku API dependency (io.github.iamr0s:Dhizuku-API:2.5.4) - Add DhizukuInstaller with binder caching for performance - Add android:testOnly flag required for Dhizuku device owner - Add Dhizuku to installer selection dialog with permission checks - Add ProGuard rules for Dhizuku classes - Add string resources for Dhizuku installer UI Credit: https://github.com/alltechdev/aurora-dhizuku
Session installer behaves identically to Native for non-system apps, as USER_ACTION_NOT_REQUIRED only works with INSTALL_PACKAGES permission.
coderabbit coderabbit coderabbit
alltechdev
force-pushed
the
feat/download-install-updates
branch
from
63ade63 to
11aa068
Compare
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
app/src/main/kotlin/com/metrolist/music/App.kt (1)
69-73:⚠️ Potential issue | 🟠 MajorNarrow hidden API exemptions to exact descriptors.
Line 72 uses
addHiddenApiExemptions("I", "L"), which is overly broad for startup-time hidden API bypass. Restrict this to the specific PM interfaces actually needed by installer paths.Suggested fix
- runCatching { - HiddenApiBypass.addHiddenApiExemptions("I", "L") - }.onFailure { + runCatching { + HiddenApiBypass.addHiddenApiExemptions( + "Landroid/content/pm/IPackageManager;", + "Landroid/content/pm/IPackageInstaller;", + "Landroid/content/pm/IPackageInstallerSession;", + ) + }.onFailure { Timber.w(it, "Hidden API bypass unavailable; privileged installers will be disabled") }#!/bin/bash set -euo pipefail # Verify current exemption call site rg -n 'addHiddenApiExemptions' app/src/main/kotlin/com/metrolist/music/App.kt -C2 # Verify which hidden PM interfaces are actually referenced by updater code rg -n --type=kt 'IPackageManager|IPackageInstaller|IPackageInstallerSession|PackageInstallerHidden|PackageManagerHidden' app/src/main/kotlin/com/metrolist/music/utils/updater -C2As per coding guidelines
**/*.kt: Follow best practices for Kotlin and Android development.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/App.kt` around lines 69 - 73, The current call to HiddenApiBypass.addHiddenApiExemptions("I", "L") in the App initialization is too broad; change it to only exempt the exact hidden PM interface descriptors required by the updater paths (replace the generic "I" and "L" entries with the specific descriptors used by your updater code such as IPackageManager, IPackageInstaller, IPackageInstallerSession or other exact JNI-style descriptors found in the updater package), keep the call wrapped in the existing runCatching/onFailure and gated by BuildConfig.UPDATER_AVAILABLE and Build.VERSION.SDK_INT >= Build.VERSION_CODES.P, and update the exemption arguments to the precise string descriptors discovered (e.g. the full interface descriptors referenced by the updater utilities) so only those interfaces are exempted.app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt (1)
394-406:⚠️ Potential issue | 🟡 MinorAdmonition bodies still skip the markdown renderer.
The warning/note card content is rendered with plain
Text, so**bold**, raw links, and@mentionsshow up literally inside the most prominent part of the changelog.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt` around lines 394 - 406, The changelog body is rendered with plain Text so markdown (bold, links, `@mentions`) appears literally; replace the plain Text usage that displays the variable content inside the Column (the Text call rendering content) with a Markdown/annotated-string renderer: parse content into an AnnotatedString (or use your existing MarkdownText/Markdown composable), render it with ClickableText/Text that supports spans, and wire up link click handling and mention styling so **bold**, raw URLs and `@mentions` render correctly instead of showing raw markdown.app/src/main/kotlin/com/metrolist/music/utils/ApkDownloader.kt (1)
53-109:⚠️ Potential issue | 🟠 MajorOnly mark a download complete after promoting a verified temp file.
This writes directly to
metrolist_update.apk, deletes any previous copy up front, andgetDownloadedApk()treats whatever is left on disk as a completed update. A failed, cancelled, or stale download can therefore come back as installable on the next visit.🛠️ Suggested fix
fun downloadApk( context: Context, downloadUrl: String, onProgress: ((DownloadState) -> Unit)? = null ): Flow<DownloadState> = flow { emit(DownloadState.Downloading(0f, 0, 0)) + val cacheDir = context.externalCacheDir ?: context.cacheDir + val apkFile = File(cacheDir, APK_FILE_NAME) + val tempApkFile = File(cacheDir, "$APK_FILE_NAME.tmp") + try { - val cacheDir = context.externalCacheDir ?: context.cacheDir - val apkFile = File(cacheDir, APK_FILE_NAME) - - // Delete existing file if any - if (apkFile.exists()) { - apkFile.delete() - } + tempApkFile.delete() val url = URL(downloadUrl) val connection = url.openConnection() as HttpURLConnection connection.requestMethod = "GET" connection.connectTimeout = 15000 @@ connection.inputStream.use { input -> - FileOutputStream(apkFile).use { output -> + FileOutputStream(tempApkFile).use { output -> val buffer = ByteArray(BUFFER_SIZE) var bytesRead: Int while (input.read(buffer).also { bytesRead = it } != -1) { output.write(buffer, 0, bytesRead) @@ } } } + if (totalBytes > 0 && downloadedBytes != totalBytes) { + tempApkFile.delete() + emit(DownloadState.Error("Download incomplete")) + return@flow + } + + if (apkFile.exists()) { + apkFile.delete() + } + if (!tempApkFile.renameTo(apkFile)) { + tempApkFile.delete() + emit(DownloadState.Error("Failed to finalize download")) + return@flow + } + emit(DownloadState.Completed(apkFile)) } catch (e: Exception) { + tempApkFile.delete() emit(DownloadState.Error(e.message ?: "Unknown error occurred")) } }.flowOn(Dispatchers.IO) @@ fun clearDownloadedApk(context: Context) { val cacheDir = context.externalCacheDir ?: context.cacheDir val apkFile = File(cacheDir, APK_FILE_NAME) + val tempApkFile = File(cacheDir, "$APK_FILE_NAME.tmp") if (apkFile.exists()) { apkFile.delete() } + if (tempApkFile.exists()) { + tempApkFile.delete() + } } fun getDownloadedApk(context: Context): File? { val cacheDir = context.externalCacheDir ?: context.cacheDir val apkFile = File(cacheDir, APK_FILE_NAME) + File(cacheDir, "$APK_FILE_NAME.tmp").delete() return if (apkFile.exists()) apkFile else null }Also applies to: 136-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/utils/ApkDownloader.kt` around lines 53 - 109, downloadApk currently writes directly to APK_FILE_NAME and deletes any prior file up front, risking a partially downloaded or failed file being treated as complete; change the implementation to write to a temporary file (e.g., APK_FILE_NAME + ".tmp" or use File.createTempFile) while streaming, only delete/replace the previous final APK_FILE_NAME after the download and integrity checks succeed, and perform an atomic rename/move from the temp file to APK_FILE_NAME inside downloadApk; reference APK_FILE_NAME, downloadApk, and getDownloadedApk to ensure getDownloadedApk only recognizes the final APK_FILE_NAME and not temp files or leftovers.app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt (1)
150-166:⚠️ Potential issue | 🟡 MinorRecovered downloads may be unreachable if release metadata is missing.
The startup path restores
downloadState = DownloadState.Completed(file)but theUpdateDownloadCardonly renders whenupdateAvailable && latestVersion != null(line 548). If the app restarts with an APK on disk but no cached release metadata, the user cannot access the install action.Consider also showing the card when
downloadState is DownloadState.Completedregardless ofupdateAvailable, or persist the version string alongside the downloaded APK.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt` around lines 150 - 166, Startup restores a completed APK via ApkDownloader.getDownloadedApk and sets downloadState = DownloadState.Completed(file) but UpdateDownloadCard only shows when updateAvailable && latestVersion != null, so restore should also make the install UI reachable: either set latestVersion and updateAvailable when you recover a downloaded APK (extract/persist a version string alongside the APK and assign latestVersion/releaseInfo/changelogContent from that persisted metadata) or change the rendering condition to show UpdateDownloadCard whenever downloadState is DownloadState.Completed (e.g., treat a recovered DownloadState.Completed as a valid path to render install actions regardless of Updater.getCachedLatestRelease()). Ensure changes reference downloadedApkFile, downloadState / DownloadState.Completed, latestVersion, updateAvailable, Updater.getCachedLatestRelease, and UpdateDownloadCard.app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt (1)
176-219:⚠️ Potential issue | 🟠 MajorAbandon install sessions on failure paths to prevent resource leaks.
After
pm install-createsucceeds, failure paths (session ID parsing, write, commit) return without callingpm install-abandon $sessionId. Stale sessions accumulate inPackageInstallerand can block future installations.The same pattern applies to
installShizuku(lines 235-306) andinstallDhizuku(lines 324-397) wheresession.abandon()is not called on failure paths before exceptions are caught.Suggested fix for installRoot
val sessionId = sessionIdMatcher.group(1)?.toInt() - ?: return InstallResult.Error("Invalid session ID") + ?: run { + Shell.cmd("pm install-abandon $sessionId").exec() + return InstallResult.Error("Invalid session ID") + } // Write APK to session val writeResult = Shell.cmd( "cat \"${apkFile.absolutePath}\" | pm install-write -S ${apkFile.length()} $sessionId \"${apkFile.name}\"" ).exec() if (!writeResult.isSuccess) { + Shell.cmd("pm install-abandon $sessionId").exec() return InstallResult.Error(writeResult.err.joinToString("\n").ifEmpty { "Failed to write APK" }) } // Commit session val commitResult = Shell.cmd("pm install-commit $sessionId").exec() if (commitResult.isSuccess) { InstallResult.Success } else { + Shell.cmd("pm install-abandon $sessionId").exec() InstallResult.Error(commitResult.err.joinToString("\n").ifEmpty { "Install commit failed" }) }For Shizuku/Dhizuku, wrap session operations in try-finally with
session.abandon()in the catch block andsession.close()in finally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt` around lines 176 - 219, After a successful pm install-create you must abandon the created session on any failure path; modify the root installer flow so that after extracting sessionId you wrap the write/commit sequence in try/catch/finally and on any early-return or exception call Shell.cmd("pm install-abandon $sessionId").exec() (or equivalent) before returning the InstallResult, and only omit abandon when commit succeeded; similarly, update installShizuku and installDhizuku to wrap session operations in try/catch/finally where catch calls session.abandon() and finally calls session.close() to ensure sessions are abandoned on errors and always closed.app/src/main/AndroidManifest.xml (1)
3-4:⚠️ Potential issue | 🔴 CriticalRemove
android:testOnly="true"from the main manifest.The
android:testOnly="true"attribute prevents the APK from being published to Google Play and restricts installation to ADB only. This attribute is still present at line 4 and must be removed from the main manifest or moved to a debug-only build variant configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/AndroidManifest.xml` around lines 3 - 4, Remove the android:testOnly="true" attribute from the main AndroidManifest XML (it's currently set on the <manifest> root) so the release APK can be published; if you need testOnly for debug builds, move that attribute or the entire manifest change into a debug-only manifest overlay or Gradle variant (e.g., src/debug/AndroidManifest.xml) instead of leaving it in the main manifest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/build.gradle.kts`:
- Around line 286-297: The updater-related libs (libs.dhizuku.api,
libs.shizuku.api, libs.libsu.core, libs.rikka.tools.refine.runtime,
libs.lsposed.hiddenapibypass, etc.) are currently added unconditionally; move
them into flavor-scoped dependency configurations for the updater-enabled
flavors so the izzy flavor (with UPDATER_AVAILABLE = false) won’t package them.
Replace the unconditional implementation/compileOnly entries with
flavor-specific configurations (e.g., <flavorName>Implementation /
<flavorName>CompileOnly) for all flavors that should include the updater,
ensuring AppInstaller.kt and UpdaterSettings.kt remain compilable only in those
updater-enabled flavors.
In `@app/src/main/kotlin/com/metrolist/music/constants/PreferenceKeys.kt`:
- Line 87: The current InstallerTypeKey uses intPreferencesKey which leads to
persisting enum ordinals; change to storing InstallerType.name by replacing
intPreferencesKey("installerType") with a string-based key (e.g.,
stringPreferencesKey("installerType")) and update all read/write sites that use
InstallerTypeKey (places that parse/save InstallerType) to read/write by name
(use InstallerType.valueOf when reading) or an explicit persisted id; also add a
one-time migration that detects an existing integer value under
InstallerTypeKey, maps that ordinal to the correct InstallerType (using
InstallerType.values()) and writes the corresponding name string before removing
the old int value so existing installs are preserved.
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt`:
- Around line 432-444: The DhizukuRequestPermissionListener.onRequestPermission
callback runs off the main thread but updates Compose state (calls
onInstallerTypeChange, sets showInstallerDialog and installError); wrap those
state updates to run on the main thread (e.g., post a Runnable to
Handler(Looper.getMainLooper()) or launch a coroutine on Dispatchers.Main) so
inside onRequestPermission call Handler(Looper.getMainLooper()).post {
onInstallerTypeChange(...); showInstallerDialog = false } or post setting
installError similarly; also ensure the catch block sets installError on the
main thread as well.
In `@app/src/main/kotlin/com/metrolist/music/utils/Updater.kt`:
- Around line 145-150: The ReleaseInfo construction is using GitHub's free-form
release title for versionName but that field is later treated as a semantic
version; change ReleaseInfo to keep the comparable version from tagName
(json.getString("tag_name")) and move the human-readable title
(json.optString("name", ...)) into a new separate field (e.g., displayName or
title) on ReleaseInfo, update the site that reads ReleaseInfo.versionName to use
tagName for comparisons, and apply the same change in getAllReleases() so both
caches store tagName as the canonical version and the release title only as the
human display field.
- Around line 174-178: getAllReleases() currently passes the raw response into
JSONArray(response), which fails when GitHub returns an error object (JSON
object with "message"/"errors") instead of an array; update the fetch logic in
getAllReleases() to inspect the body text after client.get(...) and before
JSONArray(...)—if it parses as a JSONObject containing "message" or "errors"
(same pattern used in getLatestRelease()), unwrap and throw or surface that
error (include the message and any error details) instead of attempting to
create a JSONArray, otherwise proceed to parse the array as before.
---
Duplicate comments:
In `@app/src/main/AndroidManifest.xml`:
- Around line 3-4: Remove the android:testOnly="true" attribute from the main
AndroidManifest XML (it's currently set on the <manifest> root) so the release
APK can be published; if you need testOnly for debug builds, move that attribute
or the entire manifest change into a debug-only manifest overlay or Gradle
variant (e.g., src/debug/AndroidManifest.xml) instead of leaving it in the main
manifest.
In `@app/src/main/kotlin/com/metrolist/music/App.kt`:
- Around line 69-73: The current call to
HiddenApiBypass.addHiddenApiExemptions("I", "L") in the App initialization is
too broad; change it to only exempt the exact hidden PM interface descriptors
required by the updater paths (replace the generic "I" and "L" entries with the
specific descriptors used by your updater code such as IPackageManager,
IPackageInstaller, IPackageInstallerSession or other exact JNI-style descriptors
found in the updater package), keep the call wrapped in the existing
runCatching/onFailure and gated by BuildConfig.UPDATER_AVAILABLE and
Build.VERSION.SDK_INT >= Build.VERSION_CODES.P, and update the exemption
arguments to the precise string descriptors discovered (e.g. the full interface
descriptors referenced by the updater utilities) so only those interfaces are
exempted.
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.kt`:
- Around line 394-406: The changelog body is rendered with plain Text so
markdown (bold, links, `@mentions`) appears literally; replace the plain Text
usage that displays the variable content inside the Column (the Text call
rendering content) with a Markdown/annotated-string renderer: parse content into
an AnnotatedString (or use your existing MarkdownText/Markdown composable),
render it with ClickableText/Text that supports spans, and wire up link click
handling and mention styling so **bold**, raw URLs and `@mentions` render
correctly instead of showing raw markdown.
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt`:
- Around line 150-166: Startup restores a completed APK via
ApkDownloader.getDownloadedApk and sets downloadState =
DownloadState.Completed(file) but UpdateDownloadCard only shows when
updateAvailable && latestVersion != null, so restore should also make the
install UI reachable: either set latestVersion and updateAvailable when you
recover a downloaded APK (extract/persist a version string alongside the APK and
assign latestVersion/releaseInfo/changelogContent from that persisted metadata)
or change the rendering condition to show UpdateDownloadCard whenever
downloadState is DownloadState.Completed (e.g., treat a recovered
DownloadState.Completed as a valid path to render install actions regardless of
Updater.getCachedLatestRelease()). Ensure changes reference downloadedApkFile,
downloadState / DownloadState.Completed, latestVersion, updateAvailable,
Updater.getCachedLatestRelease, and UpdateDownloadCard.
In `@app/src/main/kotlin/com/metrolist/music/utils/ApkDownloader.kt`:
- Around line 53-109: downloadApk currently writes directly to APK_FILE_NAME and
deletes any prior file up front, risking a partially downloaded or failed file
being treated as complete; change the implementation to write to a temporary
file (e.g., APK_FILE_NAME + ".tmp" or use File.createTempFile) while streaming,
only delete/replace the previous final APK_FILE_NAME after the download and
integrity checks succeed, and perform an atomic rename/move from the temp file
to APK_FILE_NAME inside downloadApk; reference APK_FILE_NAME, downloadApk, and
getDownloadedApk to ensure getDownloadedApk only recognizes the final
APK_FILE_NAME and not temp files or leftovers.
In `@app/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt`:
- Around line 176-219: After a successful pm install-create you must abandon the
created session on any failure path; modify the root installer flow so that
after extracting sessionId you wrap the write/commit sequence in
try/catch/finally and on any early-return or exception call Shell.cmd("pm
install-abandon $sessionId").exec() (or equivalent) before returning the
InstallResult, and only omit abandon when commit succeeded; similarly, update
installShizuku and installDhizuku to wrap session operations in
try/catch/finally where catch calls session.abandon() and finally calls
session.close() to ensure sessions are abandoned on errors and always closed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 743946d7-5916-410a-a30e-664425897ddb
📒 Files selected for processing (19)
app/build.gradle.ktsapp/proguard-rules.proapp/src/izzy/AndroidManifest.xmlapp/src/main/AndroidManifest.xmlapp/src/main/kotlin/com/metrolist/music/App.ktapp/src/main/kotlin/com/metrolist/music/constants/PreferenceKeys.ktapp/src/main/kotlin/com/metrolist/music/ui/component/ReleaseNotesCard.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/AccountSettings.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/ChangelogScreen.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/SettingsScreen.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.ktapp/src/main/kotlin/com/metrolist/music/utils/ApkDownloader.ktapp/src/main/kotlin/com/metrolist/music/utils/Updater.ktapp/src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.ktapp/src/main/kotlin/com/metrolist/music/utils/updater/InstallReceiver.ktapp/src/main/kotlin/com/metrolist/music/utils/updater/Installer.ktapp/src/main/res/values/metrolist_strings.xmlbuild.gradle.ktsgradle/libs.versions.toml
Comment on lines
+286
to
+297
| // Shizuku for privileged installation | ||
| compileOnly(libs.rikka.hidden.stub) | ||
| implementation(libs.rikka.tools.refine.runtime) | ||
| implementation(libs.shizuku.api) | ||
| implementation(libs.shizuku.provider) | ||
| implementation(libs.lsposed.hiddenapibypass) | ||
|
|
||
| // libsu for root access | ||
| implementation(libs.libsu.core) | ||
|
|
||
| // Dhizuku for device owner installation | ||
| implementation(libs.dhizuku.api) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm izzy flavor disables updater
rg -n 'create\("izzy"\)|UPDATER_AVAILABLE' app/build.gradle.kts -C2
# Confirm updater deps are global in current file
rg -n 'compileOnly\(libs\.rikka\.hidden\.stub\)|implementation\(libs\.(rikka\.tools\.refine\.runtime|shizuku\.api|shizuku\.provider|lsposed\.hiddenapibypass|libsu\.core|dhizuku\.api)\)' app/build.gradle.kts -C2
# Check whether updater-specific APIs are imported from src/main (if yes, source-set split/no-op abstraction is needed)
rg -n --type=kt 'import (dev\.rikka|rikka\.shizuku|moe\.shizuku|com\.topjohnwu\.superuser|com\.rosan\.dhizuku|org\.lsposed\.hiddenapibypass)' app/src/main/kotlin -C1Repository: MetrolistGroup/Metrolist
Length of output: 1343
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Retry import check without the -type flag that caused error
echo "=== Checking for updater-specific API imports ==="
rg 'import (dev\.rikka|rikka\.shizuku|moe\.shizuku|com\.topjohnwu\.superuser|com\.rosan\.dhizuku|org\.lsposed\.hiddenapibypass)' app/src/main/ -C1 || echo "No updater API imports found in src/main"
# Check for existing flavor-scoped dependency examples in build.gradle.kts
echo -e "\n=== Checking for existing flavor-scoped dependency examples ==="
rg '"(foss|gms|izzy)(Implementation|CompileOnly|RuntimeOnly)"' app/build.gradle.kts -C1 | head -50
# Verify the three flavors are the complete list
echo -e "\n=== Confirm all product flavors in the file ==="
rg 'flavorDimensions|create\(' app/build.gradle.kts | grep -E '(flavorDimensions|create\()'Repository: MetrolistGroup/Metrolist
Length of output: 2551
Scope updater installer dependencies to updater-enabled flavors only.
Lines 287-297 add privileged updater dependencies unconditionally, but the izzy flavor at Line 74 sets UPDATER_AVAILABLE = false. This causes izzy builds to unnecessarily package updater libraries (Dhizuku, Shizuku, libsu, HiddenApiBypass, Refine) despite disabling the updater feature. Updater-specific APIs are actively imported in src/main/kotlin/com/metrolist/music/utils/updater/AppInstaller.kt and UpdaterSettings.kt, so these dependencies have real usage. The codebase already uses flavor-scoped dependencies for similar cases (see gmsImplementation for Google Cast libs), establishing a precedent for this pattern.
Suggested fix
- // Shizuku for privileged installation
- compileOnly(libs.rikka.hidden.stub)
- implementation(libs.rikka.tools.refine.runtime)
- implementation(libs.shizuku.api)
- implementation(libs.shizuku.provider)
- implementation(libs.lsposed.hiddenapibypass)
-
- // libsu for root access
- implementation(libs.libsu.core)
-
- // Dhizuku for device owner installation
- implementation(libs.dhizuku.api)
+ // Updater stack only for updater-enabled flavors
+ "fossCompileOnly"(libs.rikka.hidden.stub)
+ "gmsCompileOnly"(libs.rikka.hidden.stub)
+
+ "fossImplementation"(libs.rikka.tools.refine.runtime)
+ "gmsImplementation"(libs.rikka.tools.refine.runtime)
+ "fossImplementation"(libs.shizuku.api)
+ "gmsImplementation"(libs.shizuku.api)
+ "fossImplementation"(libs.shizuku.provider)
+ "gmsImplementation"(libs.shizuku.provider)
+ "fossImplementation"(libs.lsposed.hiddenapibypass)
+ "gmsImplementation"(libs.lsposed.hiddenapibypass)
+
+ "fossImplementation"(libs.libsu.core)
+ "gmsImplementation"(libs.libsu.core)
+
+ "fossImplementation"(libs.dhizuku.api)
+ "gmsImplementation"(libs.dhizuku.api)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/build.gradle.kts` around lines 286 - 297, The updater-related libs
(libs.dhizuku.api, libs.shizuku.api, libs.libsu.core,
libs.rikka.tools.refine.runtime, libs.lsposed.hiddenapibypass, etc.) are
currently added unconditionally; move them into flavor-scoped dependency
configurations for the updater-enabled flavors so the izzy flavor (with
UPDATER_AVAILABLE = false) won’t package them. Replace the unconditional
implementation/compileOnly entries with flavor-specific configurations (e.g.,
<flavorName>Implementation / <flavorName>CompileOnly) for all flavors that
should include the updater, ensuring AppInstaller.kt and UpdaterSettings.kt
remain compilable only in those updater-enabled flavors.
| val CheckForUpdatesKey = booleanPreferencesKey("checkForUpdates") | ||
| val UpdateNotificationsEnabledKey = booleanPreferencesKey("updateNotifications") | ||
| val LastUpdateCheckTimeKey = longPreferencesKey("lastUpdateCheckTime") | ||
| val InstallerTypeKey = intPreferencesKey("installerType") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 '\bInstallerTypeKey\b' app/src/main/kotlin
rg -n -C3 '\.ordinal\b|entries\[[^]]+\]|valueOf\s*\(' app/src/main/kotlinRepository: MetrolistGroup/Metrolist
Length of output: 17461
🏁 Script executed:
fd -n "Installer.kt" app/src/main/kotlin --exec cat -n {} \;Repository: MetrolistGroup/Metrolist
Length of output: 298
🏁 Script executed:
fd "Installer.kt" app/src/main/kotlin --exec cat -n {} \;Repository: MetrolistGroup/Metrolist
Length of output: 20260
Persist installer choice with a stable identifier.
An Int key here is very likely to end up storing InstallerType ordinals. That becomes brittle as soon as the enum changes shape—the installer list already changed during this PR—so a saved value can silently point at a different backend after an update.
🛠️ Suggested direction
-val InstallerTypeKey = intPreferencesKey("installerType")
+val InstallerTypeKey = stringPreferencesKey("installerType")Store InstallerType.name (or an explicit persisted id) at the read/write sites, and migrate any existing int-backed value once.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/kotlin/com/metrolist/music/constants/PreferenceKeys.kt` at line
87, The current InstallerTypeKey uses intPreferencesKey which leads to
persisting enum ordinals; change to storing InstallerType.name by replacing
intPreferencesKey("installerType") with a string-based key (e.g.,
stringPreferencesKey("installerType")) and update all read/write sites that use
InstallerTypeKey (places that parse/save InstallerType) to read/write by name
(use InstallerType.valueOf when reading) or an explicit persisted id; also add a
one-time migration that detects an existing integer value under
InstallerTypeKey, maps that ordinal to the correct InstallerType (using
InstallerType.values()) and writes the corresponding name string before removing
the old int value so existing installs are preserved.
Comment on lines
+432
to
+444
| Dhizuku.requestPermission(object : DhizukuRequestPermissionListener() { | ||
| override fun onRequestPermission(grantResult: Int) { | ||
| if (grantResult == PackageManager.PERMISSION_GRANTED) { | ||
| onInstallerTypeChange(type.ordinal) | ||
| showInstallerDialog = false | ||
| } else { | ||
| installError = context.getString(R.string.installer_dhizuku_unavailable) | ||
| } | ||
| } | ||
| }) | ||
| } catch (e: Exception) { | ||
| installError = context.getString(R.string.installer_dhizuku_unavailable) | ||
| } |
Contributor
There was a problem hiding this comment.
Dhizuku permission callback updates Compose state from a non-main thread.
The DhizukuRequestPermissionListener.onRequestPermission callback executes on a binder thread, but onInstallerTypeChange and showInstallerDialog are Compose state variables that should be updated on the main thread. This can cause state consistency issues or crashes.
Suggested fix using Handler or coroutine
+import android.os.Handler
+import android.os.Looper
...
Dhizuku.requestPermission(object : DhizukuRequestPermissionListener() {
override fun onRequestPermission(grantResult: Int) {
- if (grantResult == PackageManager.PERMISSION_GRANTED) {
- onInstallerTypeChange(type.ordinal)
- showInstallerDialog = false
- } else {
- installError = context.getString(R.string.installer_dhizuku_unavailable)
- }
+ Handler(Looper.getMainLooper()).post {
+ if (grantResult == PackageManager.PERMISSION_GRANTED) {
+ onInstallerTypeChange(type.ordinal)
+ showInstallerDialog = false
+ } else {
+ installError = context.getString(R.string.installer_dhizuku_unavailable)
+ }
+ }
}
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/UpdaterSettings.kt`
around lines 432 - 444, The DhizukuRequestPermissionListener.onRequestPermission
callback runs off the main thread but updates Compose state (calls
onInstallerTypeChange, sets showInstallerDialog and installError); wrap those
state updates to run on the main thread (e.g., post a Runnable to
Handler(Looper.getMainLooper()) or launch a coroutine on Dispatchers.Main) so
inside onRequestPermission call Handler(Looper.getMainLooper()).post {
onInstallerTypeChange(...); showInstallerDialog = false } or post setting
installError similarly; also ensure the catch block sets installError on the
main thread as well.
Comment on lines
145
to
+150
| val releaseInfo = ReleaseInfo( | ||
| tagName = json.getString("tag_name"), | ||
| versionName = json.getString("name"), | ||
| description = json.getString("body"), | ||
| releaseDate = json.getString("published_at"), | ||
| assets = parseAssets(json.getJSONArray("assets")) | ||
| versionName = json.optString("name", json.getString("tag_name")), | ||
| description = json.optString("body", ""), | ||
| releaseDate = json.optString("published_at", ""), | ||
| assets = parseAssets(json.optJSONArray("assets") ?: org.json.JSONArray()) |
Contributor
There was a problem hiding this comment.
Keep the comparable version sourced from tag_name.
Line 147 now lets GitHub's free-form release title populate versionName. That field is later treated as a version number, so titles like Hot Fixes or 13.2.1 stable can hide a real update or report a false one.
🛠️ Suggested tweak
- versionName = json.optString("name", json.getString("tag_name")),
+ versionName = json.getString("tag_name"),If you still want the human-readable release title, store it separately instead of overloading the semantic version field. Mirror the same rule in getAllReleases() so both caches stay consistent.
📝 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.
Suggested change
| val releaseInfo = ReleaseInfo( | |
| tagName = json.getString("tag_name"), | |
| versionName = json.getString("name"), | |
| description = json.getString("body"), | |
| releaseDate = json.getString("published_at"), | |
| assets = parseAssets(json.getJSONArray("assets")) | |
| versionName = json.optString("name", json.getString("tag_name")), | |
| description = json.optString("body", ""), | |
| releaseDate = json.optString("published_at", ""), | |
| assets = parseAssets(json.optJSONArray("assets") ?: org.json.JSONArray()) | |
| val releaseInfo = ReleaseInfo( | |
| tagName = json.getString("tag_name"), | |
| versionName = json.getString("tag_name"), | |
| description = json.optString("body", ""), | |
| releaseDate = json.optString("published_at", ""), | |
| assets = parseAssets(json.optJSONArray("assets") ?: org.json.JSONArray()) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/kotlin/com/metrolist/music/utils/Updater.kt` around lines 145 -
150, The ReleaseInfo construction is using GitHub's free-form release title for
versionName but that field is later treated as a semantic version; change
ReleaseInfo to keep the comparable version from tagName
(json.getString("tag_name")) and move the human-readable title
(json.optString("name", ...)) into a new separate field (e.g., displayName or
title) on ReleaseInfo, update the site that reads ReleaseInfo.versionName to use
tagName for comparisons, and apply the same change in getAllReleases() so both
caches store tagName as the canonical version and the release title only as the
human display field.
Comment on lines
+174
to
178
| val response = client.get("$GITHUB_API_BASE/releases?page=$page&per_page=30") { | ||
| header(HttpHeaders.UserAgent, "Metrolist/${BuildConfig.VERSION_NAME}") | ||
| header(HttpHeaders.Accept, "application/vnd.github+json") | ||
| }.bodyAsText() | ||
| val json = JSONArray(response) |
Contributor
There was a problem hiding this comment.
Handle GitHub error objects in the paginated fetch too.
getLatestRelease() now unwraps API error payloads, but getAllReleases() still feeds the raw body straight into JSONArray. When GitHub rate-limits or rejects the request, this path throws a parse error and the changelog falls back to looking empty instead of surfacing the real failure.
🛠️ Suggested tweak
val response = client.get("$GITHUB_API_BASE/releases?page=$page&per_page=30") {
header(HttpHeaders.UserAgent, "Metrolist/${BuildConfig.VERSION_NAME}")
header(HttpHeaders.Accept, "application/vnd.github+json")
}.bodyAsText()
- val json = JSONArray(response)
+ val trimmedResponse = response.trimStart()
+ if (trimmedResponse.startsWith("{")) {
+ val error = JSONObject(trimmedResponse)
+ if (error.has("message")) {
+ throw Exception(error.getString("message"))
+ }
+ }
+ val json = JSONArray(response)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/kotlin/com/metrolist/music/utils/Updater.kt` around lines 174 -
178, getAllReleases() currently passes the raw response into
JSONArray(response), which fails when GitHub returns an error object (JSON
object with "message"/"errors") instead of an array; update the fetch logic in
getAllReleases() to inspect the body text after client.get(...) and before
JSONArray(...)—if it parses as a JSONObject containing "message" or "errors"
(same pattern used in getLatestRelease()), unwrap and throw or surface that
error (include the message and any error details) instead of attempting to
create a JSONArray, otherwise proceed to parse the array as before.
Took another look at the policy, it looks like that wouldn't even be needed as long as it's not enabled by default (perhaps a prompt on first launch asking users if they want to enable it would be appropriate in this case) The part of the policy in reference:
|
Member
|
@radioaddition i added the izzyondroid build that does NOT include any updater elements/modules |
3 tasks
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
5 participants
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.
Summary
Installation Methods
Changes
ApkDownloader.ktutility with Kotlin Flow-based progressAppInstaller.ktwith multiple installer implementationsUpdaterSettings.ktwith download/install UI and installer selection dialogUpdater.ktMarkdownTextnow supports>[!WARNING],>[!NOTE], etc.Credits
Test plan
Testing note
To test the update functionality, temporarily bump down the version in
app/build.gradle.kts(e.g., changeversionNameto"13.2.0"andversionCodeto141), then build and install. The app will detect the latest release as an available update.Screenshots
screen-compressed.mp4
Summary by CodeRabbit
New Features
UI
Localization
Chores