Fix daily limits, feature-only blocking, the DM-reel bypass and #15's wrong timers - #17
Open
polubarev wants to merge 5 commits into
Open
Fix daily limits, feature-only blocking, the DM-reel bypass and #15's wrong timers#17polubarev wants to merge 5 commits into
polubarev wants to merge 5 commits into
Conversation
Daily limits were evaluated against a number that was structurally always
zero, so they never fired.
EvaluateBlockUseCase fed BlockEngine a dailyUsageMs from
UsageRepository.getDailyUsage(), which is SUM(durationMs) over the Room
usage_events table. That table logs block/allow DECISIONS and nothing ever
writes durationMs — ScreenTimeProvider already says so in a comment. The sum
is therefore 0 for every package, forever.
Two consequences:
- `dailyUsageMs >= limit` is permanently false, so the daily-limit
HARD_BLOCK never fired;
- dailyTimeRemainingMs was always the full limit, so the overlay's
"X left today" never moved. That is the reported symptom.
The non-obvious part: TimeRemainingHandler reads the correct source
(UsageStatsManager) and enforces the limit itself, so daily limits appeared
to work — but ONLY for rules that had opted into the time-remaining overlay.
For everyone else the feature was silently dead.
Route all three call sites through one dailyUsageMs() helper on
getDailyForegroundTimeMs, and delete getDailyUsage so the trap cannot be
re-introduced. The read is a binder call, hence Dispatchers.IO; it returns 0
when Usage Access is not granted, which fails toward ALLOWING the app — a
permission the user never gave must not manufacture a block.
Verified: with the fix reverted, 4 of the 6 new tests fail. The other two
("budget not yet spent", "missing usage access") are guards that must pass
either way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reported on-device: a user configured YouTube and Instagram expecting
Shorts/Reels-only blocking and got both apps blocked entirely. It was not a
misconfiguration — the app had no way to express it.
UnifiedAppConfigScreen is an "app-level default + per-feature overrides"
model, and performSave ALWAYS writes the app-level rule because it carries
dailyLimitMinutes, showCounter, showTimeRemaining, grayscale and webDomains.
With only three blocking modes that rule always gated the app, and
FeatureMode.INHERIT means "follow the app default", never "leave the app
alone". Feature overrides could only make a feature DIFFER from a block that
always existed.
BlockMode.NONE resolves it without restructuring: the app-level rule is still
written and still carries those settings, but contributes no block.
BlockEngine needed NO logic change — NONE matches none of its block branches.
mode is stored as a String, so no DB migration.
- A daily limit on a NONE rule is still enforced: the time-budget branch
keys off dailyLimitMinutes, not the mode. "Don't gate this app, but stop
me after 30 min" is a real thing users want.
- UI is a "Block the whole app" switch, not a fourth segment. A segment
labelled "Don't block" beside "Hard Block" is the same ambiguity that
caused the original report. lastBlockingMode restores the prior choice
when toggled back on rather than snapping to DELAY.
- NONE never appears in a mode picker. Both pickers use explicit lists.
For a SCHEDULED override it would be a lie: scheduled rules are additive,
so a NONE scheduled rule adds nothing rather than carving out a free
window.
- Strict Mode gates it. Switching a blocking rule to NONE is the largest
weakening this editor can produce; RuleWeakening lists "NONE" -> 0
explicitly rather than relying on the unknown-mode fallback.
- BlockOverlayActivity finishes early on NONE, so it can never render a
block screen for an app the user chose not to gate.
Device-verified: YouTube now opens freely while Shorts hard-blocks.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A reel opened from a DM could be scrolled indefinitely with a HARD_BLOCK rule active on REELS. Reels opened from the Reels TAB blocked correctly, which is why the gap went unnoticed. Instagram's full-screen reel player is hosted in com.instagram.modal.ModalActivity, which has NO bottom navigation. detectInstagram resolved the feature purely by asking which nav tab was `selected`, so the player was structurally invisible — not mis-detected, undetectable in principle. Detect the player's own containers (clips_viewer_view_pager / clips_video_container / clips_media_component) BEFORE any tab reasoning, so every entry route is covered — DM, share link, profile, and the Reels tab itself, where the same containers are present. IDs were harvested from a real device (Galaxy S24 / Android 16) and verified absent from the home feed (inline video sits under media_group / carousel_video_media_group) and from a DM thread, so they do not over-match ordinary browsing. Several containers are listed because the player's tree varies by entry point — the DM variant additionally carries a reply bar. Any one suffices. Also adds a debug-only diagnostic that logs the view-ids of surfaces where detection failed. This is what found the bug: the usual tools cannot see these screens, because `uiautomator dump` waits for an idle window and a playing reel never idles (it hangs and is Killed), while `dumpsys activity top` times out on the same screens. It logs each DISTINCT surface once — detection runs on a firehose (~800 failed detections in three minutes of measured use) — and reads ONLY viewIdResourceName, never text or contentDescription, which on these screens would be private messages. An earlier attempt in this session also routed TYPE_VIEW_SCROLLED into detection, on the theory that the player emits few content-change events. Measurement disproved it: ~26k content-changes against ~1.8k scrolls. That change is not included; detection opportunity was never the bottleneck. Device-verified: reels opened from a DM now block. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reported: "it played a timer titled 'Goguma' displayed a 30s timer (time for Discord ...) the timer counted down, yet the circle did not move." BlockOverlayActivity is singleInstance, so a block arriving while the overlay is already up is delivered via onNewIntent. render() then calls setContent again — but setContent on an EXISTING ComposeView reuses the composition, and every `remember` slot sits at the same call position and survives. appLabel and delaySeconds are parameters and update; the countdown state does not. So the overlay showed the NEW app's name over the PREVIOUS app's remaining seconds, and the ring froze because progress = remainingSeconds/delaySeconds went off-scale (30 remaining over a 15s delay = 2.0, clamped full). Every detail of the report follows from that one cause. Key the whole subtree on a per-delivery token so a new block starts from clean state. Done once in the activity rather than by keying individual `remember` calls: it covers all three overlays at once, and keying on delaySeconds — the obvious choice — is NOT sufficient, because two apps both on the default 15s delay would still hand each other stale state. The token is a counter, not a hash of the block's contents: re-delivering the SAME block is still a new attempt that must start from full. Also keeps the in-flight improvement that ticks the "X left today" line down alongside the countdown instead of showing a frozen snapshot. Not unit-testable: this is Compose composition identity across onNewIntent, which needs a UI-test harness the project does not have. Device-verified on a Galaxy S24 — two apps with different delays, second block delivered over a live countdown, correct name + correct seconds + moving ring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The diagnostic added alongside the reel-player fix deduplicated its LOGGING but not its WALK, so it still BFS'd up to 800 nodes on every failed detection — ~800 times in three minutes of measured Instagram use — directly on the accessibility hot path. Fine for a capture session, not fine for a build someone runs all day. A new surface stays on screen far longer than the interval, so nothing is missed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Six commits from a day of device QA on a Galaxy S24 / Android 16. Every change is unit-tested and device-verified; 595 tests pass.
Happy to split this into separate PRs, drop anything, or adjust naming — say the word.
fix(#14): the daily budget was spent against a column nothing writesEvaluateBlockUseCasefedBlockEngineadailyUsageMsfromgetDailyUsage(), which isSUM(durationMs)overusage_events. That table logs block/allow decisions and nothing ever writesdurationMs—ScreenTimeProvideralready says so in a comment. So the value is 0 for every package, forever:dailyUsageMs >= limitis permanently false, so the daily-limit HARD_BLOCK never fireddailyTimeRemainingMswas always the full limit, so "X left today" never movedThe subtle part:
TimeRemainingHandlerreads the correct source and enforces the limit itself, so daily limits appeared to work — but only for rules that had opted into the time-remaining overlay. For everyone else the feature was silently dead.Routed through
getDailyForegroundTimeMsand deletedgetDailyUsageso the trap can't return. Verified by reverting the fix: 4 of the 6 new tests fail.feat:BlockMode.NONE— "block only Shorts/Reels" was not configurableReported by a user who configured YouTube and Instagram expecting Shorts/Reels-only blocking and got both apps blocked entirely. Not a misconfiguration — the app couldn't express it.
performSavealways writes the app-level rule (it carriesdailyLimitMinutes,showCounter,showTimeRemaining,grayscale,webDomains), andFeatureMode.INHERITmeans "follow the app default", never "leave the app alone".NONEkeeps that rule as a settings carrier while contributing no block.BlockEngineneeded no logic change;modeis a String column so no migration. A daily limit on a NONE rule is still enforced. Strict Mode gates switching to it.UI is a "Block the whole app" switch rather than a fourth mode segment — "Don't block" beside "Hard Block" is the same ambiguity that caused the original report.
fix: Instagram's reel player, not just the bottom-nav tabA reel opened from a DM could be scrolled indefinitely with HARD_BLOCK on REELS. Reels from the Reels tab blocked fine, which hid it.
The player runs in
com.instagram.modal.ModalActivity— no bottom navigation — anddetectInstagramresolved the feature purely from which tab wasselected. The player was undetectable in principle.Now checks the player's own containers first, covering every entry route. IDs harvested from a real device and verified absent from the home feed and DM threads.
Worth flagging: tab detection appears dead on current Instagram — no tab reported
selected=truein any capture (~800 detection attempts, all null). The player check is currently carrying Instagram detection alone.Includes a debug-only view-id harvest, because the usual tools can't see these screens:
uiautomator dumpwaits for an idle window and a playing reel never idles (hangs, then Killed);dumpsys activity toptimes out. Logs each distinct unrecognised surface once, one tree walk per 5s, and reads onlyviewIdResourceName— never text or contentDescription, which here would be private messages.fix(#15): each delivered block gets its own compositionBlockOverlayActivityissingleInstance, so a second block arrives viaonNewIntent→setContent— which reuses the composition, so everyrememberslot survives.appLabelanddelaySecondsare parameters and update; the countdown state doesn't. Hence the new app's name over the old app's seconds, and a frozen ring (30/15 = 2.0, off-scale).Keyed on a per-delivery token. Keying on
delaySecondsis not enough — two apps both on the default 15s would still share state.Not unit-testable (Compose composition identity across
onNewIntent); device-verified instead.feat: order Manage Apps by rule statusThe list showed every installed app unsorted, so configured apps were scattered through hundreds of untouched ones. Now: active rules, then apps whose rule is off, then the rest — alphabetical within each.
Also found but not fixed here (happy to file as issues): PiP defeats feature blocking — YouTube enters Picture-in-Picture when the overlay backgrounds it and the Short keeps playing. Root cause is platform behaviour (pinned tasks are always-on-top; the overlay is correctly fullscreen and
topResumedActivityand still loses). The app-op fix is device-verified: turning off Picture-in-picture for YouTube removes the bypass. Nudge can't set that app-op itself, so the honest shape is detect-and-deep-link toandroid.settings.PICTURE_IN_PICTURE_SETTINGS.🤖 Generated with Claude Code