Installer attribution (#444 ask 3/3)#504
Merged
Merged
Conversation
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:
WalkthroughAdds installer-attribution support: new domain model and validation, persistence in TweaksRepositoryImpl, UI controls and ViewModel handling, expanded AIDL/service APIs and implementations to accept an installer package name, and dispatcher wiring so silent installs pass the chosen installer identity. ChangesInstaller Attribution Feature
Sequence DiagramsequenceDiagram
participant User
participant UI as Installation UI
participant VM as TweaksViewModel
participant Repo as TweaksRepository
participant Dispatcher as SilentInstallerDispatcher
participant Service as Shizuku/Dhizuku Service
User->>UI: Choose attribution (preset/custom)
UI->>VM: Dispatch attribution action
VM->>Repo: setInstallerAttribution(...)
Repo-->>VM: persisted
User->>UI: Request silent install
UI->>Dispatcher: trySilentInstall(...)
Dispatcher->>Repo: read cached attribution
Repo-->>Dispatcher: InstallerAttribution
Dispatcher->>Dispatcher: resolvePackageName()
Dispatcher->>Service: installPackage(..., installerPackageName)
Service-->>Dispatcher: result
Dispatcher-->>UI: success/failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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 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 |
![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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/InstallerAttribution.kt`:
- Around line 35-42: The current Validator in InstallerAttributionDefaults
allows uppercase package names because packageNamePattern was created with
RegexOption.IGNORE_CASE; remove that flag so the regex enforces lowercase-only
package segments (keep the pattern string as-is) and keep isValidPackageName
using the same trimmed input. Update packageNamePattern (and any related usage)
to use a case-sensitive regex so values like "Com.Android.Vending" no longer
validate via InstallerAttributionDefaults.isValidPackageName.
- Around line 16-20: resolvePackageName() returns raw Custom.packageName which
can include surrounding whitespace; update the Custom branch to normalize by
trimming before checking/returning (e.g., use packageName.trim().takeIf {
it.isNotBlank() }) so callers like SilentInstallerDispatcher and install
backends never receive leading/trailing spaces while preserving the existing
SystemDefault -> null and Preset -> key.packageName behavior.
In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Installation.kt`:
- Line 217: Replace the hard-coded placeholder Text("com.example.installer") in
Installation.kt with a localized string from the core/presentation localization
system (e.g., use the app's localization provider or stringResource and a new
key such as installer_default_package or installationDefaultPackage on the
shared Strings object) and update the 13-language resource files to include
translations for that key; ensure the placeholder uses the localized value
(e.g., Text(localizedStrings.installer_default_package) or
Text(stringResource(R.string.installer_default_package)) rather than the literal
"com.example.installer").
In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt`:
- Around line 303-316: observeInstallerAttribution currently updates
installerAttribution and installerAttributionCustomDraft but never toggles
installerAttributionCustomExpanded, so the custom editor can stay visible when a
non-Custom attribution (e.g., SystemDefault or a preset) is received; inside the
collect block for tweaksRepository.getInstallerAttribution() update the state
copy to set installerAttributionCustomExpanded = if (attribution is
zed.rainxch.core.domain.model.InstallerAttribution.Custom) keep
current.installerAttributionCustomExpanded else false, ensuring the custom
editor collapses whenever the persisted attribution is not Custom while
preserving expansion when it remains Custom.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4218d9c7-7173-4815-93da-3d943f9084d6
📒 Files selected for processing (26)
core/data/src/androidMain/aidl/zed/rainxch/core/data/services/dhizuku/IDhizukuInstallerService.aidlcore/data/src/androidMain/aidl/zed/rainxch/core/data/services/shizuku/IShizukuInstallerService.aidlcore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/dhizuku/DhizukuInstallerServiceImpl.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/installer/SilentInstallerDispatcher.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/shizuku/ShizukuInstallerServiceImpl.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/TweaksRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/InstallerAttribution.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.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.jsoncore/presentation/src/commonMain/composeResources/values/strings.xmlfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksAction.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksState.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Installation.kt
| onValueChange = { onAction(TweaksAction.OnInstallerAttributionCustomChanged(it)) }, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| label = { Text(stringResource(Res.string.installer_attribution_custom_label)) }, | ||
| placeholder = { Text("com.example.installer") }, |
Contributor
There was a problem hiding this comment.
Localize the custom package placeholder.
com.example.installer is user-facing text, so it should come from resources rather than being hard-coded.
Suggested fix
- placeholder = { Text("com.example.installer") },
+ placeholder = { Text(stringResource(Res.string.installer_attribution_custom_placeholder)) },As per coding guidelines, Localize all user-facing strings using the 13-language localization system provided by core/presentation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Installation.kt`
at line 217, Replace the hard-coded placeholder Text("com.example.installer") in
Installation.kt with a localized string from the core/presentation localization
system (e.g., use the app's localization provider or stringResource and a new
key such as installer_default_package or installationDefaultPackage on the
shared Strings object) and update the 13-language resource files to include
translations for that key; ensure the placeholder uses the localized value
(e.g., Text(localizedStrings.installer_default_package) or
Text(stringResource(R.string.installer_default_package)) rather than the literal
"com.example.installer").
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt`:
- Around line 661-679: OnInstallerAttributionCustomSave currently launches
tweaksRepository.setInstallerAttribution(...) without error handling and clears
installerAttributionCustomError synchronously, risking crashes and premature
clearing; change the handler so you launch a coroutine with runCatching around
tweaksRepository.setInstallerAttribution(...) (use viewModelScope.launch {
runCatching { tweaksRepository.setInstallerAttribution(...) } .onSuccess {
_state.update { it.copy(installerAttributionCustomError = null) } } .onFailure {
_state.update { it.copy(installerAttributionCustomError = "write_failed" /* or
map error */) }; log the exception } }), ensuring the error state is only
cleared on success and exceptions are caught to avoid uncaught crashes; keep
observeInstallerAttribution() behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33f4e205-cf2d-4f58-9dac-0f698285d079
📒 Files selected for processing (2)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/InstallerAttribution.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
✅ Files skipped from review due to trivial changes (1)
- core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/InstallerAttribution.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt`:
- Around line 615-641: Handlers OnInstallerAttributionSystemDefault and
OnInstallerAttributionPresetSelected call setInstallerAttribution inside
viewModelScope.launch without runCatching and perform the optimistic
_state.update outside the write success path; wrap the setInstallerAttribution
call in runCatching inside the coroutine, move the optimistic state mutation
(clearing error and collapsing installerAttributionCustomExpanded) into the
runCatching.onSuccess block, and handle failures in onFailure (or log) so
IOExceptions from DataStore are caught and the UI state remains consistent;
reference these symbols: OnInstallerAttributionSystemDefault,
OnInstallerAttributionPresetSelected, setInstallerAttribution, _state.update,
installerAttributionCustomExpanded, runCatching, viewModelScope.launch.
- Around line 303-322: The observer in TweaksViewModel that does
viewModelScope.launch { getInstallerAttribution().collect { ... } } can die if
getInstallerAttribution() throws; wrap the flow with .catch { e -> /* log via
logger or _uiState update and optionally emit a fallback/default value */ }
before collect so the coroutine isn't cancelled on error. Apply the same pattern
to the sibling observers (observeShizukuStatus(), observeDhizukuStatus()): use
flow.catch to handle/log errors and supply a sensible fallback or no-op
emission, then continue with collect to update state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e620fc1c-8daf-4d9c-9c8f-12c46616fd22
📒 Files selected for processing (1)
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
rainxchzed
force-pushed
the
feat/444-installer-attribution
branch
from
214f588 to
9126258
Compare
…tom value, collapse editor on non-custom
…e to user instead of clearing error eagerly
…and guard installer attribution flow with catch
rainxchzed
force-pushed
the
feat/444-installer-attribution
branch
from
9126258 to
f68579f
Compare
This was referenced May 6, 2026
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.
Third of three PRs for #444. Lets users override the installer-of-record name attached to silent installs (Shizuku, Dhizuku) so apps that gate on installer source can be coaxed into running. See plan in `roadmap/444_C_INSTALLER_ATTRIBUTION.md`.
What's in here
What's deferred (Phase 2)
Plan C originally proposed both global default and per-app override (in `AdvancedAppSettingsBottomSheet`). Per-app needs a Room migration adding `installerAttributionOverride: String?` to `installed_apps` — bigger surface than this PR's scope deserves. Phase 1 ships global-only today. Per-app override + the install-failure-sheet contextual hint land in a follow-up.
Tradeoffs
Test plan
Summary by CodeRabbit
New Features
Chores