Cooldown Feature for digital addictions - #204
Open
TaqiHamoda wants to merge 14 commits into
Open
Conversation
|
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 lockdown mode with configurable cooldown timing, clock-tampering detection, and guards for VPN stop, pause, and toggle actions. Adds device-owner support for always-on VPN and user restrictions, settings controls, persistence, validation, and localized resources. ChangesLockdown mode and device-owner controls
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant LockdownScreen
participant BlockAdsApp
participant AppPreferences
participant AdBlockVpnService
User->>LockdownScreen: Start cooldown
LockdownScreen->>BlockAdsApp: Report cooldown timestamp
BlockAdsApp->>AppPreferences: Persist cooldown and activity timestamps
User->>AdBlockVpnService: Request stop or pause
AdBlockVpnService->>AppPreferences: Read lockdownEnabled
AdBlockVpnService->>AdBlockVpnService: Ignore action while locked
LockdownScreen->>BlockAdsApp: Report completion or tampering
BlockAdsApp->>AppPreferences: Clear cooldown or disable lockdown
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 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 |
Contributor
Author
|
Here are some screenshots from my phone regarding what the implemented feature looks like:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/pwhs/blockads/data/datastore/AppPreferences.kt`:
- Around line 161-163: The lockdownDuration Flow property does not validate that
stored durations are within allowed preset values, allowing invalid durations
(0, negative, or non-preset values) that can weaken cooldown behavior. Add
validation at both the write-time (around line 412 where the duration is set)
and read-time (in the lockdownDuration Flow mapping) to normalize any incoming
duration to the closest allowed preset duration. First define the allowed preset
durations as constants, then apply this normalization logic in both the setter
operation and the map transformation of the lockdownDuration Flow to ensure
consistency.
In `@app/src/main/java/app/pwhs/blockads/service/AdBlockTileService.kt`:
- Around line 38-43: The onClick() callback in AdBlockTileService is blocking
the SystemUI thread by using runBlocking to read from appPrefs.lockdownEnabled.
Replace the runBlocking call with a non-blocking coroutine-based approach by
launching a coroutine using an appropriate scope (such as viewModelScope or
lifecycleScope if available in the tile service context). Read the
lockdownEnabled value asynchronously and then perform the updateTileState() and
return logic within the coroutine without blocking the caller. Apply the same
fix to line 49 where runBlocking is also used.
In `@app/src/main/java/app/pwhs/blockads/service/AdBlockVpnService.kt`:
- Around line 272-286: Replace the blocking `runBlocking {
appPrefs.lockdownEnabled.first() }` calls in the onStartCommand method with
non-blocking coroutine launches using the available serviceScope. Instead of
synchronously reading the lockdownEnabled preference and blocking execution,
launch an async coroutine using serviceScope that checks the lockdownEnabled
state and performs the appropriate action (either returning early with Timber
logging or calling stopVpn). Ensure the onStartCommand method returns
START_STICKY immediately to indicate the service will continue processing
asynchronously, rather than blocking the main thread waiting for the DataStore
read to complete. This pattern should be applied at both occurrences where
lockdownEnabled is currently being read in a blocking manner.
In `@app/src/main/java/app/pwhs/blockads/service/RootProxyService.kt`:
- Around line 151-164: The onStartCommand callback is using runBlocking on the
main thread to check appPrefs.lockdownEnabled for both the ACTION_STOP and
ACTION_PAUSE_1H cases, which can block service command handling. Replace these
blocking calls with a non-blocking coroutine approach: use a coroutine scope
(such as lifecycleScope.launch or a custom scope tied to the service) to
asynchronously check the lockdown status, and move the conditional logic that
calls stopProxy() and returns START_STICKY or START_NOT_STICKY inside the
coroutine block. Ensure the service returns START_REDELIVER_INTENT or another
appropriate value immediately from onStartCommand while the async check happens
in the background.
In `@app/src/main/java/app/pwhs/blockads/ui/LockdownScreen.kt`:
- Around line 150-156: The durationText construction in the LockdownScreen.kt
file hardcodes English strings "hour", "hours", and "minutes" instead of using
localized resources. Move these string literals to Android string resources so
they can be properly localized for different languages. The hour/minute duration
labels should be extracted from the durationText variable and replaced with
references to localized string resources, or reuse any localized duration label
utilities that were introduced elsewhere in this PR to maintain consistency.
- Around line 31-50: The lastActiveTime variable retains its initial value
across idle periods and countdown cycles. When a countdown starts after the user
has been on the screen for more than 5 minutes, the check at line 45 comparing
the time difference incorrectly triggers tampering detection. Reset
lastActiveTime to System.currentTimeMillis() immediately after entering the
LaunchedEffect block (after verifying cooldownStart > 0L) so the tamper
detection logic uses a fresh baseline from when the countdown actually begins,
not from when the user initially loaded the screen.
In `@app/src/main/java/app/pwhs/blockads/ui/settings/SettingsViewModel.kt`:
- Around line 148-154: The setLockdownEnabled function has a race condition
where the service start decision relies on the passed-in enabled parameter
instead of the latest persisted state. If the user quickly toggles ON→OFF, a
stale coroutine could still call ServiceController.requestStart even though the
state is already OFF. After calling appPrefs.setLockdownEnabled(enabled),
re-check the current persisted state from appPrefs (using the appropriate getter
method) before the if condition that calls ServiceController.requestStart,
rather than using the enabled parameter directly to ensure the most recent
stored state is used for the service control decision.
In `@app/src/main/res/values-iw/strings.xml`:
- Line 535: The Hebrew translation for singular durations in the
lockdown_duration_1m string (line 535) and the corresponding hour duration
string at line 539 has reversed word order that reads unnaturally in Hebrew.
Correct the string values by placing the number before the unit name instead of
after it, so that "דקה 1" becomes the proper singular form and "שעה 1" is
similarly corrected. Apply this same fix to both duration strings to ensure
natural Hebrew phrasing in the UI.
In `@app/src/main/res/values/strings.xml`:
- Line 580: Move the duration formatting logic from LockdownScreen.kt out of the
hardcoded English text construction and replace it with localized string
resources that handle pluralization properly. Create separate plural string
resources for hours and minutes in the strings.xml file, then update the code
that constructs durationText to use these localized resources instead of
concatenating hardcoded English units like "hour(s)" and "minutes". This ensures
the entire formatted duration string passed to the lockdown_screen_duration
placeholder will be properly localized for all languages.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f645f49e-d1d9-43c9-8f81-082a19405df7
📒 Files selected for processing (31)
app/src/main/java/app/pwhs/blockads/data/datastore/AppPreferences.ktapp/src/main/java/app/pwhs/blockads/service/AdBlockTileService.ktapp/src/main/java/app/pwhs/blockads/service/AdBlockVpnService.ktapp/src/main/java/app/pwhs/blockads/service/BootReceiver.ktapp/src/main/java/app/pwhs/blockads/service/RootProxyService.ktapp/src/main/java/app/pwhs/blockads/service/ServiceController.ktapp/src/main/java/app/pwhs/blockads/ui/BlockAdsApp.ktapp/src/main/java/app/pwhs/blockads/ui/LockdownScreen.ktapp/src/main/java/app/pwhs/blockads/ui/settings/SettingsScreen.ktapp/src/main/java/app/pwhs/blockads/ui/settings/SettingsViewModel.ktapp/src/main/java/app/pwhs/blockads/ui/settings/component/LockdownSection.ktapp/src/main/java/app/pwhs/blockads/widget/WidgetToggleReceiver.ktapp/src/main/res/values-ar/strings.xmlapp/src/main/res/values-cs/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-in/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-iw/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-th/strings.xmlapp/src/main/res/values-tr/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-vi/strings.xmlapp/src/main/res/values-zh/strings.xmlapp/src/main/res/values/strings.xml
Member
|
thanks I will check it |
Contributor
Author
Thank you for all the hard work and for this amazing app! One small question, should I implement the suggestions that code rabbit made? |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/pwhs/blockads/BlockAdsApplication.kt`:
- Around line 62-65: The startup enforcement in BlockAdsApplication should
respect the user’s saved restriction state instead of reapplying restrictions
whenever DeviceOwnerManager reports device-owner mode. Update the app launch
flow around the device-owner check to consult the persisted enforcement toggle
before calling DeviceOwnerManager.enforceRestrictions(), or remove the ability
to clear restrictions if persistence is not supported. Use the existing
DeviceOwnerManager and the app startup logic to locate the change.
In `@app/src/main/java/app/pwhs/blockads/service/DeviceOwnerManager.kt`:
- Around line 19-23: areRestrictionsEnforced() is only validating one
device-owner restriction, so the reported state can look complete even when
other policy pieces are missing. Update DeviceOwnerManager’s enforcement flow so
enforceRestrictions() and the clear path return success/failure for each
expected policy step, then have areRestrictionsEnforced() verify all required
restrictions and always-on VPN state, including DISALLOW_APPS_CONTROL,
DISALLOW_CONFIG_VPN, and DISALLOW_DEBUGGING_FEATURES, before returning true.
In
`@app/src/main/java/app/pwhs/blockads/ui/settings/component/DeviceOwnerSection.kt`:
- Around line 41-44: The DeviceOwnerSection lockdown state only dims the
SettingsToggleItem visually, but the toggleable remains interactive. Update
SettingsToggleItem to accept an enabled flag and wire it through to its
toggleable behavior, then pass !lockdownEnabled from DeviceOwnerSection so the
control is semantically disabled for accessibility and keyboard input when
lockdown is active.
In `@app/src/main/java/app/pwhs/blockads/ui/settings/SettingsViewModel.kt`:
- Around line 490-498: `SettingsViewModel.setRestrictionsEnforced` currently
updates `_restrictionsEnforced` from the requested flag instead of the real
device policy state, so the UI can become stale when
`deviceOwnerManager.enforceRestrictions()` or `clearRestrictions()` no-op. After
calling the manager, re-read the state with
`deviceOwnerManager.areRestrictionsEnforced()` (or change `DeviceOwnerManager`
to return the actual Boolean result) and assign that value to
`_restrictionsEnforced` inside the `viewModelScope.launch(Dispatchers.IO)`
block.
In `@app/src/main/res/values-th/strings.xml`:
- Line 555: Update the localized string for
settings_device_owner_enforce_restrictions_desc to remove the misleading “clear
data” wording and instead state that disabling will clear or remove the enforced
restrictions only. Keep the meaning aligned with the existing restrictions
toggle in the same resource entry, so users understand it affects app
restrictions rather than app/device data.
In `@app/src/main/res/values-tr/strings.xml`:
- Line 576: Update the localized string for
settings_device_owner_enforce_restrictions_desc so the disable action is
explicit: keep the meaning that turning it off clears enforced restrictions, and
avoid wording that could imply clearing user data. Adjust the text in the
translation entry to clearly reference disabling enforced restrictions rather
than “clearing” generically.
In `@FEATURES.md`:
- Around line 34-466: The FEATURES.md links are using machine-local
file:///home/... paths, which won’t work for reviewers. Replace those absolute
links with repo-relative markdown links throughout the document, using the
existing target file names and paths referenced in the diff (for example the
SettingsScreen.kt, BlockAdsApp.kt, AppPreferences.kt, and service classes) so
the links resolve correctly in the repository.
- Line 255: The uninstall guidance is inconsistent with the behavior of
setUninstallBlocked(admin, packageName, true), which blocks removal in Settings
and via adb uninstall. Update the uninstall-related wording in the FEATURES.md
sections tied to App Uninstallation and the DO removal guidance so they all
describe the actual supported removal path, or remove the claim that advanced
users can still uninstall with standard ADB commands.
- Line 253: The VPN revocation row in FEATURES.md has a mismatch between the
stated behavior and the `setAlwaysOnVpnPackage(admin, packageName,
lockdownEnabled = false)` call. Update the `System VPN Revocation` description
to either pass `true` in `setAlwaysOnVpnPackage` if lockdown is intended, or
remove the lockdown/no-bypass wording from both references so the documented
security claim matches the API usage.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ce79516-510f-455c-9000-8e1c1a9e5315
📒 Files selected for processing (30)
CONCLUSION.mdFEATURES.mdNOTES.mdapp/src/main/AndroidManifest.xmlapp/src/main/java/app/pwhs/blockads/BlockAdsApplication.ktapp/src/main/java/app/pwhs/blockads/service/AdBlockDeviceAdminReceiver.ktapp/src/main/java/app/pwhs/blockads/service/DeviceOwnerManager.ktapp/src/main/java/app/pwhs/blockads/ui/settings/SettingsScreen.ktapp/src/main/java/app/pwhs/blockads/ui/settings/SettingsViewModel.ktapp/src/main/java/app/pwhs/blockads/ui/settings/component/DeviceOwnerSection.ktapp/src/main/res/values-ar/strings.xmlapp/src/main/res/values-cs/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-in/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-iw/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-th/strings.xmlapp/src/main/res/values-tr/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-vi/strings.xmlapp/src/main/res/values-zh/strings.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/xml/device_admin_policies.xml
✅ Files skipped from review due to trivial changes (12)
- app/src/main/res/xml/device_admin_policies.xml
- NOTES.md
- app/src/main/res/values-it/strings.xml
- app/src/main/res/values-ja/strings.xml
- app/src/main/res/values-ar/strings.xml
- CONCLUSION.md
- app/src/main/res/values-de/strings.xml
- app/src/main/res/values-uk/strings.xml
- app/src/main/res/values-cs/strings.xml
- app/src/main/res/values-vi/strings.xml
- app/src/main/res/values-pl/strings.xml
- app/src/main/res/values-ko/strings.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/res/values-iw/strings.xml
- app/src/main/java/app/pwhs/blockads/ui/settings/SettingsScreen.kt
- app/src/main/res/values-zh/strings.xml
Contributor
Author
|
So to help with understanding the new changes and the motivation for this PR, I have made Gemini do a short write-up of the changes: BlockAds Feature Specification: VPN Timer Lock (Lockdown Mode)This document specifies the technical design, workflow, code changes, and edge-case handling for the VPN Lockdown Mode & Cooldown Timer feature. 🎯 Feature Overview & MotivationFor users dealing with digital addictions (such as gambling, social media, shopping, or adult content), the standard ad-blocker suffers from an architectural limitation: the user can easily turn it off during a moment of weakness. In a split second, an impulsive urge can lead the user to open settings, whitelist a domain, or disable the VPN entirely, bypassing their own self-imposed protection. The VPN Timer Lock (Lockdown Mode) introduces cognitive friction to interrupt this immediate feedback/reward loop. By locking down settings and disabling direct shutdown switches, the app forces a delayed cooling-off period. To disable the protection, the user must initiate a countdown (e.g., 30 minutes). During this time, they cannot browse blocked sites, but they are given time to cool down, self-reflect, and ideally let the superficial urge pass. 🔄 User Experience (UX) FlowstateDiagram-v2
[*] --> Unlocked : Default State
Unlocked --> SettingsScreen : Open settings
SettingsScreen --> Locked : Enable VPN Lockdown + Set Cooldown (e.g., 30m)
state Locked {
[*] --> LockOverlayActive : App Launched
LockOverlayActive --> CooldownRunning : Click "Start Cooldown Timer"
CooldownRunning --> LockOverlayActive : Click "Cancel Cooldown" (Resets timer)
CooldownRunning --> CooldownRunning : Countdown ticking down
}
CooldownRunning --> Unlocked : Timer reaches zero (Resets Settings)
🛠️ Technical Design & Code MappingTo ensure a foolproof lockdown, we must intercept VPN shutdown actions across all entry points and enforce the lockout screen overlay. 1. Data Layer ConfigurationWe introduce three state variables in // AppPreferences.kt additions
val lockdownEnabled: Flow<Boolean> = dataStore.data.map { it[LOCKDOWN_ENABLED] ?: false }
val lockdownDuration: Flow<Long> = dataStore.data.map { it[LOCKDOWN_DURATION] ?: 300000L } // default 5 minutes
val cooldownStartTimestamp: Flow<Long> = dataStore.data.map { it[COOLDOWN_START_TIMESTAMP] ?: 0L }
suspend fun setLockdownEnabled(enabled: Boolean) {
dataStore.edit { it[LOCKDOWN_ENABLED] = enabled }
}
suspend fun setLockdownDuration(ms: Long) {
dataStore.edit { it[LOCKDOWN_DURATION] = ms }
}
suspend fun setCooldownStartTimestamp(timestamp: Long) {
dataStore.edit { it[COOLDOWN_START_TIMESTAMP] = timestamp }
}2. Lockout UI Overlay (Compose root)To prevent the user from accessing setting views, we overlay a Compose layout at the root container // BlockAdsApp.kt UI layout composition
@Composable
fun BlockAdsApp(...) {
val appPrefs: AppPreferences = koinInject()
val isLocked by appPrefs.lockdownEnabled.collectAsState(initial = false)
val cooldownStart by appPrefs.cooldownStartTimestamp.collectAsState(initial = 0L)
val duration by appPrefs.lockdownDuration.collectAsState(initial = 300000L)
Box(modifier = Modifier.fillMaxSize()) {
// Main App Navigation
NavDisplay(backStack = backStack, ...)
// Root Lockdown Overlay
if (isLocked) {
LockdownScreen(
cooldownStart = cooldownStart,
duration = duration,
onStartCooldown = { timestamp ->
coroutineScope.launch { appPrefs.setCooldownStartTimestamp(timestamp) }
},
onCancelCooldown = {
coroutineScope.launch { appPrefs.setCooldownStartTimestamp(0L) }
},
onUnlockComplete = {
coroutineScope.launch {
appPrefs.setLockdownEnabled(false)
appPrefs.setCooldownStartTimestamp(0L)
}
}
)
}
}
}LockdownScreen Ticker LogicInside @Composable
fun LockdownScreen(
cooldownStart: Long,
duration: Long,
onStartCooldown: (Long) -> Unit,
onCancelCooldown: () -> Unit,
onUnlockComplete: () -> Unit
) {
var currentTime by remember { mutableStateOf(System.currentTimeMillis()) }
// LaunchedEffect ticker running every 1 second
LaunchedEffect(cooldownStart) {
if (cooldownStart > 0L) {
while (true) {
currentTime = System.currentTimeMillis()
val elapsed = currentTime - cooldownStart
if (elapsed >= duration) {
onUnlockComplete()
break
}
delay(1000L)
}
}
}
// Calculate remaining seconds
val remainingMs = if (cooldownStart > 0L) duration - (currentTime - cooldownStart) else duration
val secondsLeft = (remainingMs / 1000).coerceAtLeast(0)
// Render lockdown UI, warnings, lock icons, progress bar, and trigger buttons...
}3. Service Lifecycle Guarding (Failsafe)We prevent external or programmatic stop commands inside // Inside AdBlockVpnService.onStartCommand / stopVpn checks
override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
if (intent?.action == ACTION_STOP) {
val isLocked = runBlocking { appPrefs.lockdownEnabled.first() }
if (isLocked) {
Timber.w("Stop request ignored: VPN is in Lockdown Mode.")
return START_STICKY
}
}
// standard command routing...
}Similarly, in 4. Integration Broadcasts & InterceptorsWe intercept requests from remote widgets, Tasker, and tiles:
🛡️ Edge Cases & Handling Mitigations1. Clock Manipulation (Time Tampering)Scenario: The user starts a 12-hour cooldown timer and then shifts the Android system calendar clock forward 12 hours to force completion. Mitigations:
2. Device RebootsScenario: The user restarts the device to clean volatile states or break loop checking. Mitigations:
3. Settings Bypass via Back Press / GesturesScenario: The user attempts to dismiss the Lockout screen via back navigation or system gestures. Mitigations:
4. TV Platform LimitationsScenario: How does the TV Companion App handle the lock? Mitigations:
📈 Standard Sandbox Limitations vs. Enterprise EnforcementIn a standard Android sandboxed environment, a regular application is constrained by the OS security model. Without administrative privileges, the following bypass actions cannot be fully prevented at the system level:
While the standard Lockdown Mode relies on cognitive friction (adding steps to interrupt impulsive urges), advanced users or individuals experiencing strong urges can bypass it by performing these OS-level actions. To address these vulnerabilities and offer an unbreakable impulse control mechanism, BlockAds can be configured in Android Device Owner (DO) Mode. This leverages Android Enterprise APIs to enforce system-level compliance, effectively blocking the standard bypass vectors. 🏢 Android Device Owner (DO) Mode: Unbreakable Impulse ControlAndroid Device Owner (DO) mode allows BlockAds to act as the device's Device Policy Controller (DPC). By enrolling the app with elevated administrative privileges, we can call privileged APIs under 🤫 Hidden Activation & Auto-Enforcement DesignTo prevent average users from being confused or accidentally lock-in their settings, DO Mode is designed as a hidden, auto-enforcing state:
🔄 Mitigation Matrix: Resolving standard bypasses
🛠️ Proposed Architecture & Code IntegrationIntegrating Device Owner capabilities requires minimal overhead because it utilizes built-in Android system framework classes, but it requires strict lifecycle guarding. graph TD
Startup[App Startup / Service Boot] -->|Check Device Owner Status| DPM_Check{isDeviceOwnerApp?}
DPM_Check -->|Yes| EnforcePolicies[Automatically Apply DPM restrictions]
DPM_Check -->|No| NormalMode[Run in Standard Sandbox Mode]
subgraph UISettings ["UI Settings (Only Visible if Device Owner)"]
DO_UIVisible[Render DO Settings section] --> ClearRest[Clear Restrictions Button]
ClearRest -->|Only in Unlocked State| RunClear[Suspend restrictions]
end
subgraph SystemRestrictions ["System Restrictions"]
EnforcePolicies -->|1. Lock VPN| AlwaysOn[Always-on VPN + Lockdown]
EnforcePolicies -->|2. Lock Package| UninstallBlocked[setUninstallBlocked = true]
EnforcePolicies -->|3. Disallow Settings| RestrictVPN[DISALLOW_CONFIG_VPN]
EnforcePolicies -->|4. Close ADB Bypass| RestrictDebug[DISALLOW_DEBUGGING_FEATURES]
end
1. Component Registration:
|
There was a problem hiding this comment.
Actionable comments posted: 2
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/java/app/pwhs/blockads/ui/LockdownScreen.kt (1)
116-116: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLocalize or remove the decorative icon description.
"Locked"bypasses the localized string resources; usestringResource(...)orcontentDescription = nullif the title already conveys the state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/pwhs/blockads/ui/LockdownScreen.kt` at line 116, The decorative icon in LockdownScreen still uses a hardcoded contentDescription of "Locked" instead of localized resources. Update the icon’s contentDescription in the LockdownScreen composable to either use stringResource(...) with the existing string resources or set it to null if the nearby title already communicates the locked state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/pwhs/blockads/ui/LockdownScreen.kt`:
- Around line 70-89: Use a cumulative monotonic baseline for cooldown tracking
in LockdownScreen’s cooldown loop. The current drift check updates
lastActiveTime/lastActiveRealtime every tick, which lets repeated small
wall-clock jumps accumulate and still unlock via elapsed; instead, keep the
effect’s initial wall-time and realtime pair as the baseline for
onTimeTamperingDetected checks, and compute the cooldown completion from
monotonic elapsed since that baseline rather than current wall-clock elapsed.
- Around line 45-54: The persisted tamper check in LockdownScreen’s
LaunchedEffect currently skips validation when elapsedRealtime() has reset after
reboot because initialReal is less than lastPersistedRealtime. Update the logic
around initialReal, lastPersistedRealtime, and onTimeTamperingDetected so
rebooted monotonic baselines are treated as untrusted: cancel and restart the
cooldown or reinitialize the persisted baseline before evaluating wall-clock
differences, instead of bypassing the check. Make the fix in the same
tamper-detection block that compares initialWall, lastPersistedTime, and
realDiff.
---
Outside diff comments:
In `@app/src/main/java/app/pwhs/blockads/ui/LockdownScreen.kt`:
- Line 116: The decorative icon in LockdownScreen still uses a hardcoded
contentDescription of "Locked" instead of localized resources. Update the icon’s
contentDescription in the LockdownScreen composable to either use
stringResource(...) with the existing string resources or set it to null if the
nearby title already communicates the locked 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54a36f89-2127-43d2-bedc-6369af4cbb58
📒 Files selected for processing (4)
app/src/main/java/app/pwhs/blockads/data/datastore/AppPreferences.ktapp/src/main/java/app/pwhs/blockads/service/DeviceOwnerManager.ktapp/src/main/java/app/pwhs/blockads/ui/BlockAdsApp.ktapp/src/main/java/app/pwhs/blockads/ui/LockdownScreen.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/app/pwhs/blockads/ui/BlockAdsApp.kt
- app/src/main/java/app/pwhs/blockads/service/DeviceOwnerManager.kt
- app/src/main/java/app/pwhs/blockads/data/datastore/AppPreferences.kt
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/test/java/app/pwhs/blockads/LockdownDurationValidationTest.kt (1)
27-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the
AppPreferencesbehavior, not only the constant.These assertions only restate membership in
ALLOWED_LOCKDOWN_DURATIONS. They do not verify thatsetLockdownDurationnormalizes an invalid value or thatlockdownDurationfalls back toDEFAULT_LOCKDOWN_DURATION.Add DataStore-backed tests for both paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/app/pwhs/blockads/LockdownDurationValidationTest.kt` around lines 27 - 44, Replace the membership-only assertions in testInvalidDurationsRejected and testValidDurationsAccepted with DataStore-backed tests exercising AppPreferences.setLockdownDuration and lockdownDuration. Verify invalid inputs are normalized to DEFAULT_LOCKDOWN_DURATION, and valid inputs are persisted and returned unchanged through the lockdownDuration property.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/pwhs/blockads/ui/LockdownScreen.kt`:
- Around line 83-85: Update the LockdownScreen timing state so the computed
totalElapsed value is stored in Compose state and drives the displayed
remainingMs countdown and progress calculation. Replace the wall-clock-based UI
calculations near remainingMs with values derived from totalElapsed, while
preserving the existing monotonic unlock condition.
- Around line 45-51: Update onStartCooldown’s initial persistence path to
atomically write setLastActiveTimestamp and setLastActiveRealtime in a single
DataStore.edit operation, preventing LockdownScreen from observing only one
baseline after interruption. Preserve the existing cooldown-start timestamp
behavior and ensure the countdown starts only after the combined write
completes.
---
Nitpick comments:
In `@app/src/test/java/app/pwhs/blockads/LockdownDurationValidationTest.kt`:
- Around line 27-44: Replace the membership-only assertions in
testInvalidDurationsRejected and testValidDurationsAccepted with
DataStore-backed tests exercising AppPreferences.setLockdownDuration and
lockdownDuration. Verify invalid inputs are normalized to
DEFAULT_LOCKDOWN_DURATION, and valid inputs are persisted and returned unchanged
through the lockdownDuration property.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 009ac540-efef-49c1-bdc1-9363e8662f8b
📒 Files selected for processing (11)
app/src/main/java/app/pwhs/blockads/data/datastore/AppPreferences.ktapp/src/main/java/app/pwhs/blockads/service/DeviceOwnerManager.ktapp/src/main/java/app/pwhs/blockads/ui/LockdownScreen.ktapp/src/main/java/app/pwhs/blockads/ui/settings/SettingsViewModel.ktapp/src/main/java/app/pwhs/blockads/ui/settings/component/DeviceOwnerSection.ktapp/src/main/java/app/pwhs/blockads/ui/settings/component/SettingsToggleItem.ktapp/src/main/res/values-iw/strings.xmlapp/src/main/res/values-th/strings.xmlapp/src/main/res/values-tr/strings.xmlapp/src/main/res/values/strings.xmlapp/src/test/java/app/pwhs/blockads/LockdownDurationValidationTest.kt
🚧 Files skipped from review as they are similar to previous changes (6)
- app/src/main/res/values-iw/strings.xml
- app/src/main/res/values/strings.xml
- app/src/main/res/values-th/strings.xml
- app/src/main/java/app/pwhs/blockads/ui/settings/SettingsViewModel.kt
- app/src/main/res/values-tr/strings.xml
- app/src/main/java/app/pwhs/blockads/data/datastore/AppPreferences.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/pwhs/blockads/BlockAdsApplication.kt`:
- Around line 60-61: Update the Device Owner initialization flow around
deviceOwnerManager.enforceRestrictions() to handle its false result instead of
discarding it. When enforcement fails, either retry using the existing
enforcement lifecycle or update the relevant preference/state and user-facing
status to indicate Device Owner Mode is not enforced; preserve the current path
when enforcement succeeds.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 139703ce-26ab-4664-b114-04fbbd79c347
📒 Files selected for processing (24)
app/src/main/java/app/pwhs/blockads/BlockAdsApplication.ktapp/src/main/java/app/pwhs/blockads/data/datastore/AppPreferences.ktapp/src/main/java/app/pwhs/blockads/ui/BlockAdsApp.ktapp/src/main/java/app/pwhs/blockads/ui/LockdownScreen.ktapp/src/main/java/app/pwhs/blockads/ui/settings/SettingsViewModel.ktapp/src/main/res/values-ar/strings.xmlapp/src/main/res/values-cs/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-in/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-iw/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-th/strings.xmlapp/src/main/res/values-tr/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-vi/strings.xmlapp/src/main/res/values-zh/strings.xmlapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (22)
- app/src/main/res/values/strings.xml
- app/src/main/res/values-es/strings.xml
- app/src/main/res/values-it/strings.xml
- app/src/main/res/values-iw/strings.xml
- app/src/main/java/app/pwhs/blockads/ui/BlockAdsApp.kt
- app/src/main/res/values-uk/strings.xml
- app/src/main/res/values-ko/strings.xml
- app/src/main/res/values-ja/strings.xml
- app/src/main/res/values-ru/strings.xml
- app/src/main/res/values-vi/strings.xml
- app/src/main/res/values-pt-rBR/strings.xml
- app/src/main/res/values-tr/strings.xml
- app/src/main/res/values-de/strings.xml
- app/src/main/res/values-fr/strings.xml
- app/src/main/res/values-ar/strings.xml
- app/src/main/res/values-th/strings.xml
- app/src/main/res/values-cs/strings.xml
- app/src/main/res/values-in/strings.xml
- app/src/main/res/values-pl/strings.xml
- app/src/main/java/app/pwhs/blockads/ui/LockdownScreen.kt
- app/src/main/res/values-zh/strings.xml
- app/src/main/java/app/pwhs/blockads/ui/settings/SettingsViewModel.kt
Contributor
Author
|
I have been daily driving the app on my main phone for the past month with zero issues popping up. I have been using both the cool down timer and the device admin restriction every day for the past month, enabling and disabling them and stress testing them in general. Given that I finally had some free time to get around implementing the enhancements code rabbit suggested, I will install the app with the latest changes and daily drive it for another month. I stress tested the latest changes on an Android Go 14 device with no problems. I will install it on my Pixel 7 with Android 17 installed and see if any issues pop up throughout the next month. I will write down an update here with my findings in September. |
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
2 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.
For users dealing with digital addictions (such as gambling, social media, shopping, or adult content), the standard ad-blocker suffers from an architectural limitation: the user can easily turn it off during a moment of weakness. In a split second, an impulsive urge can lead the user to open settings, whitelist a domain, or disable the VPN entirely, bypassing their own self-imposed protection.
The VPN Timer Lock (Lockdown Mode) introduces cognitive friction to interrupt this immediate feedback/reward loop. By locking down settings and disabling direct shutdown switches, the app forces a delayed cooling-off period. To disable the protection, the user must initiate a countdown (e.g., 30 minutes). During this time, they cannot browse blocked sites, but they are given time to cool down, self-reflect, and ideally let the superficial urge pass.
Summary by CodeRabbit
New Features
Bug Fixes