feat(notify): materialize the aw-notify enable flag on upgrade

[TimeToBuildBob]

Phase 4 of the aw-notify opt-in architecture (ActivityWatch/activitywatch#1435): make the shared enabled flag explicit on Android.

Draft: depends on ActivityWatch/aw-server-rust#816 (setSetting JNI). The aw-server-rust submodule bump goes into this PR once that merges. Until then the native symbol is missing and the migration call would fail. Device verification is still required before this draft is ready.

Problem

Android treats a missing aw-notify setting as default-on (preserved in #329). The already-pinned webui panel reads a missing enabled as off. Existing installs therefore see “Notifications: off” while alerts fire; fresh installs are not opt-in.

Change

A one-shot migration on service start writes the flag:

Stored aw-notify	Written
missing, upgraded install	{"enabled":true}
missing, fresh install	{"enabled":false}
object with alerts, without enabled	same object plus "enabled":true
object without alerts, explicit flag, legacy array, malformed value	untouched

• Install classification uses lastUpdateTime > firstInstallTime; lookup failure preserves upgrade behavior.
• Explicit enabled=true without alerts selects default alerts, matching the canonical Rust setting. Explicit alerts=[] suppresses alerts. This intentionally changes the previously fail-closed enabled-without-alerts shape; {} and unrelated-key objects remain fail-closed.
• The worker checks the migration-complete flag before constructing its native backend or reading settings/activity. Pending migration returns retry, including for previously scheduled workers, preventing fresh-install alerts before opt-out.
• Completion is recorded only after successful migration (or a setting that needs no write).

Known limitations before readiness

• Failed migration keeps notifications blocked until successful service recreation; automatic same-service migration retry is not implemented.
• Install timestamps alone do not reliably distinguish restored existing data from a fresh install. Backup/restore behavior is not verified.
• No native submodule bump or device/emulator verification yet.

Verification

Fix commit 4ad9d20b2a76 addresses both original Greptile findings: empty-object promotion and startup consent race.

• Both new regression tests failed before the production fix and passed afterward.
• 33 NotifyWorker tests pass.
• Full :mobile:testStandardDebugUnitTest: 263 tests across 27 suites, zero failures/errors.
• In-band AI review completed: no P0/P1 findings; P2 dispositions recorded on their threads.
• Original Greptile threads replied to and resolved; one re-review requested using the guarded helper after the verified push. New-head CI/review remain pending.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] No confirmed blocking issue remains in the reviewed changes.

Summary

Makes the shared aw-notify choice explicit on service startup.

• Android writes a clear notification choice into the shared setting.
• Notification checks wait until the setting migration finishes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    S[Service starts] --> M[Read and migrate aw-notify]
    M --> C{Write succeeds or no write needed?}
    C -- Yes --> F[Record migration complete]
    C -- No --> P[Leave migration pending]
    W[Notification worker starts] --> G{Migration complete?}
    G -- No --> R[Retry without reading settings or activity]
    G -- Yes --> A[Read settings and evaluate enabled alerts]

Loading

_{Reviews (3) · Last reviewed commit: "build(deps): bump aw-server-rust to incl..." · Reviewed by Greptile}

[TimeToBuildBob]

🤖 AI code review

Safe to merge — no P0/P1 findings on latest review

_{Updated after inline dispositions on finding threads — this is the current state; the verdict below is frozen at review time and is kept as the historical record of that pass.}

Finding disposition

Finding	Severity	State
mobile/src/main/java/net/activitywatch/android/workers/NotifyWorker.kt:236	P2	accepted-tradeoff
mobile/src/main/java/net/activitywatch/android/workers/NotifyWorker.kt:100	P2	accepted-tradeoff
mobile/src/test/java/net/activitywatch/android/workers/NotifyWorkerExecutionTest.kt:87	P2	rejected

Needs a look — P2 only

Confidence 4/5

3 findings · ⚠️ 3 P2

⚠️ P2 medium — mobile/src/main/java/net/activitywatch/android/workers/NotifyWorker.kt:236

NotifyWorker.doWork() now returns Result.retry() whenever hasMigratedNotifyOptIn() is false, but that flag is only ever set inside migrateNotifyOptIn(), which runs in BackgroundService's migration coroutine and is skipped without setting the flag when awaitRustInterface() returns null (BackgroundService.kt:178) or when setSetting's result does not start with 'ok' (lines 294-295). migrationsQueued stays true for the lifetime of the service instance, and a retrying WorkManager worker never re-runs that coroutine. For an upgrade user whose aw-notify setting is missing — previously alertsFromSetting("null") returned DEFAULT_ALERTS, so alerts fired — a single failed migration attempt leaves the gate closed: the worker retries on every backoff cycle, posts no notifications, and alerts stay silently disabled until the service is killed and recreated and the migration then succeeds. Pre-PR a migration failure was harmless because the worker still read the missing setting as defaults-on; the gate converts a tolerated failure into user-visible alert loss.

_{How this was verified: Traced the flag lifecycle: setNotifyOptInMigrated() is called only in BackgroundService.migrateNotifyOptIn() (BackgroundService.kt:289,295), which runs inside a CoroutineScope(Dispatchers.IO).launch block (BackgroundService.kt:177-193). The worker gate at NotifyWorker.kt:236-239 returns retry() without any fallback or timeout. Checked the test file: NotifyWorkerExecutionTest.kt only exercises the path where the flag is later set.}

⚠️ P2 medium — mobile/src/main/java/net/activitywatch/android/workers/NotifyWorker.kt:100

The new alertsFromSetting() branch treats an enabled config without an alerts key as DEFAULT_ALERTS, but the migration writes {"enabled":true} for upgrades with a missing setting. This means an upgrade user who had never configured alerts (the common case) now gets the default alert set, which is the intended behavior. However, the same branch also applies to a config that explicitly has {"enabled":true} but no alerts key — which is a valid user configuration that previously failed closed (returned emptyList). The PR changes that contract: a user who deliberately set enabled=true with no alerts (perhaps to suppress all alerts while keeping the toggle on) now receives the default alerts. The observable consequence is that such users start getting notifications they explicitly did not configure. The test alertsFromSetting_enabledWithoutAlertsKeyUsesDefaults() asserts the new behavior, but it does not distinguish between the migration-written value and a user-authored value, so the test passes vacuously for the user-authored case.

_{How this was verified: Traced the previous behavior: before this PR, an object with enabled=true and no alerts key would fall through to config.getJSONArray("alerts") and throw, caught by the outer try, returning emptyList. The new branch returns DEFAULT_ALERTS. The migration only writes enabled=true for missing settings (NotifyWorker.kt:120-121) or for objects with alerts (NotifyWorker.kt:129-130), so the new branch is reachable by user-authored configs too.}

⚠️ P2 medium — mobile/src/test/java/net/activitywatch/android/workers/NotifyWorkerExecutionTest.kt:87

The test pendingMigrationRetriesWithoutReadingSettingsOrActivity() removes the hasMigratedNotifyOptIn key, runs the worker, and asserts it returns retry() with no backend calls. This is a valid test of the gate. However, the @before method markMigrationComplete() sets the flag for every test, and the test itself removes it only for the first half. The second half of the test sets the flag again and runs a disabled worker. The test does not verify that the worker actually retries indefinitely or that the migration flag is the only thing preventing execution — it only checks one retry. More importantly, the test does not cover the scenario where the migration coroutine in BackgroundService is interrupted, which is the actual failure mode described in the first finding. The test is not vacuous, but it gives a false sense of coverage for the retry path because it only exercises a single retry and then immediately completes the migration.

_{How this was verified: Read the test: it removes the flag, runs the worker once, asserts retry, then sets the flag and runs again. It does not simulate a service kill or a migration that never completes. The @before sets the flag for all other tests, so the gate is not exercised elsewhere.}

1 advisory finding (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 medium — mobile/src/main/java/net/activitywatch/android/BackgroundService.kt:303

isUpgradeInstall() reduces 'existing user' to lastUpdateTime > firstInstallTime, so a device migration that restores app data via Android Auto Backup — equal install times on the new device, but SharedPreferences, the filesDir datastore, and the prior missing/default-on aw-notify state are restored — is classified as a fresh install. notifyOptInMigrationValue() then writes {"enabled":false} and sets hasMigratedNotifyOptIn, so NotifyWorker reads an explicit opt-out and sends no alerts. Pre-PR the restored user's missing setting produced DEFAULT_ALERTS; post-PR the same restored user silently loses alerts on the new device, contradicting the migration's stated goal that upgraded installs keep default-on alerts. The install-time heuristic cannot tell a restored existing user from a genuinely new one.

_{How this was verified: Checked the Android PackageManager contract: firstInstallTime is the original install time and lastUpdateTime is the last update time; a backup restore preserves both timestamps, so they remain equal. The migration uses this to decide the default enabled flag (BackgroundService.kt:283-296). The test suite does not cover the restore scenario.}

_{Reviewed 4ad9d20b2a76 · openrouter/deepseek/deepseek-v4-flash-0731 · llm (in-band) engine · 945s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

ActivityWatch/aw-server-rust#816 (setSetting JNI) merged at 2026-10-14T10:17Z. Bumped aw-server-rust to 919cbff (c74667f) — this resolves the UnsatisfiedLinkError that was crashing both the R8 smoke test and E2E tests. CI should pass the native symbol checks now.

PR remains draft pending device verification (as noted in the description).

[TimeToBuildBob]

Device verification on an x86_64 emulator (API 36, aw256-api36), done before un-drafting. The APKs are standardDebug builds of this branch at c74667f and of master at 4c13e1b, both using the CI jniLibs artifact from run 38550054719 (ActivityWatch/aw-server-rust#816 included) and the same debug key.

Case	Logcat	Stored settings.aw-notify	hasMigratedNotifyOptIn
Upgrade: master installed and run, then this build with install -r	aw-notify opt-in migration wrote {"enabled":true}: ok	{"enabled":true}	true
Restart after the upgrade (force-stop, relaunch)	no migration line	unchanged	true
Fresh install of this build	aw-notify opt-in migration wrote {"enabled":false}: ok	{"enabled":false} (read from a copy of sqlite.db after the batch commit)	true

isUpgradeInstall() correctly distinguishes the two (lastUpdateTime > firstInstallTime only on the upgrade). No UnsatisfiedLinkError and no panics. CI is green on all jobs, including E2E and the R8 release smoke.

Durability window (non-blocking, follow-up): the datastore worker does not force a commit on SetKeyValue, so a setSetting that returns ok is batched for up to ~15 s, while setNotifyOptInMigrated() persists immediately. If the process dies in that window on the first start, the migration is marked done but the key is still missing. Upgraded users then keep their pre-migration behavior (alerts on). A fresh install would get alerts on instead of opt-in. Narrow edge case, and HTTP POST /settings has the same window. The clean fix is a forced commit in the JNI setSetting on the aw-server-rust side, not here. Taking this out of draft.