diff --git a/LICENSE b/LICENSE index 7434247..325e1e7 100644 --- a/LICENSE +++ b/LICENSE @@ -1,21 +1,8 @@ -MIT License +The author disclaims copyright to this source code. In place of a legal notice, here is a blessing: -Copyright (c) 2024 Matthew Raymer +May you do good and not evil. +May you find forgiveness for yourself and forgive others. +May you share freely, never taking more than you give. -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. \ No newline at end of file +________________________________________________________________ +from https://www.sqlite.org/src/info/689401a6cfb4c234 and memorialized here https://spdx.org/licenses/blessing.html diff --git a/README.md b/README.md index 3c5f2a0..311d48a 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ This is to support apps that allow users to own their data. This approach is in * Peer-to-peer network scenarios are not supported. -There are two types of notifications supported: +There are two types of notifications supported, static & API: * Periodic static reminder messages diff --git a/android/src/main/java/org/timesafari/dailynotification/DailyNotificationPlugin.kt b/android/src/main/java/org/timesafari/dailynotification/DailyNotificationPlugin.kt index 98ffe24..5c8bf91 100644 --- a/android/src/main/java/org/timesafari/dailynotification/DailyNotificationPlugin.kt +++ b/android/src/main/java/org/timesafari/dailynotification/DailyNotificationPlugin.kt @@ -699,9 +699,13 @@ open class DailyNotificationPlugin : Plugin() { Log.i(TAG, "Cancelling all notifications") // 1. Get all scheduled notifications from database + // Includes "static_reminder" (isolated kind, see PLAN-simplify-static-and-refactor.md + // §3.1) alongside "notify" so cancelAllNotifications() genuinely cancels everything. val schedules = getDatabase().scheduleDao().getAll() - val notifySchedules = schedules.filter { it.kind == "notify" && it.enabled } - + val notifySchedules = schedules.filter { + (it.kind == "notify" || it.kind == "static_reminder") && it.enabled + } + // 2. Cancel all AlarmManager alarms (delegate to ScheduleHelper) // Only cancel alarms we can prove we scheduled (from database) // This prevents accidental cancellation of other alarms and false confidence @@ -726,9 +730,10 @@ open class DailyNotificationPlugin : Plugin() { try { // Delegate to ScheduleHelper val disabledNotify = ScheduleHelper.disableAllSchedulesByKind(getDatabase(), "notify") + val disabledStaticReminder = ScheduleHelper.disableAllSchedulesByKind(getDatabase(), "static_reminder") val disabledFetch = ScheduleHelper.disableAllSchedulesByKind(getDatabase(), "fetch") - - Log.i(TAG, "Disabled $disabledNotify notification schedule(s) and $disabledFetch fetch schedule(s)") + + Log.i(TAG, "Disabled $disabledNotify notification schedule(s), $disabledStaticReminder static reminder schedule(s), and $disabledFetch fetch schedule(s)") } catch (e: Exception) { Log.e(TAG, "Failed to clear database state", e) // Continue - alarms and jobs are already cancelled @@ -825,14 +830,24 @@ open class DailyNotificationPlugin : Plugin() { return } NotifyReceiver.cancelNotification(context, scheduleId = reminderId) + // Pure single-row delete: now that static reminders are isolated to their own + // "static_reminder" kind (see PLAN-simplify-static-and-refactor.md §3.1), there is + // guaranteed to be at most one row to deal with, so cancel can fully remove it + // ("cancel means gone") rather than leaving a disabled-but-present row that the + // rearm chain could still find and resurrect. See plan §3.3 for the deleteById vs + // setEnabled tradeoff discussion. try { kotlinx.coroutines.runBlocking { val db = getDatabase() - db.scheduleDao().setEnabled(reminderId, false) - db.scheduleDao().updateRunTimes(reminderId, null, null) + db.scheduleDao().deleteById(reminderId) + try { + db.notificationContentDao().deleteNotification(reminderId) + } catch (contentErr: Exception) { + Log.w(TAG, "cancelDailyReminder: failed to delete notification content for $reminderId", contentErr) + } } } catch (dbErr: Exception) { - Log.w(TAG, "cancelDailyReminder: failed DB update for $reminderId", dbErr) + Log.w(TAG, "cancelDailyReminder: failed DB delete for $reminderId", dbErr) } call.resolve() } catch (e: Exception) { @@ -1196,30 +1211,14 @@ open class DailyNotificationPlugin : Plugin() { // If user provides an ID, use it; otherwise use stable "daily_notification" val scheduleId = options.getString("id") ?: "daily_notification" Log.i(TAG, "scheduleDailyNotification: START - time=$time, scheduleId=$scheduleId") - - // CRITICAL: Cancel and delete all existing notification schedules before creating new one - // This ensures "one per day" semantics - only one daily notification schedule exists - // Delegate cleanup to ScheduleHelper - val cleanedCount = ScheduleHelper.cleanupExistingNotificationSchedules( - context, - getDatabase(), - excludeScheduleId = scheduleId - ) - - if (cleanedCount > 0) { - Log.i(TAG, "scheduleDailyNotification: ✅ Cleaned up $cleanedCount existing notification schedule(s) before creating new one") - } else { - Log.i(TAG, "scheduleDailyNotification: No cleanup needed - existing schedule will be updated via upsert: $scheduleId") - } - - // Cancel only fetch-related WorkManager jobs so they cannot create a second (UUID) alarm - // with fallback or placeholder text. Does not cancel display/dismiss; future fetched-content - // flows should use distinct tags so they are not affected. - val workCancelled = ScheduleHelper.cancelFetchRelatedWorkManagerJobs(context) - if (workCancelled) { - Log.i(TAG, "scheduleDailyNotification: Cancelled pending prefetch/fetch WorkManager jobs") - } - + + // No sweep/cleanup needed here: static reminders live in their own isolated + // "static_reminder" kind (see PLAN-simplify-static-and-refactor.md §3.1) and + // ScheduleHelper.scheduleDailyNotification() below does a direct upsert by the + // stable scheduleId, so there is nothing else in that kind to collide with. + // Static reminders also never enqueue prefetch/fetch WorkManager jobs, so there + // is nothing for cancelFetchRelatedWorkManagerJobs() to clean up either. + val config = UserNotificationConfig( enabled = true, schedule = cronExpression, @@ -1423,56 +1422,6 @@ open class DailyNotificationPlugin : Plugin() { } } - @PluginMethod - fun scheduleUserNotification(call: PluginCall) { - try { - if (context == null) { - return call.reject("Context not available") - } - - // Check if exact alarms can be scheduled - if (!canScheduleExactAlarms(context)) { - // Delegate to PermissionManager to handle exact alarm permission request - val activity = activity - if (activity == null) { - return call.reject("Activity not available") - } - permissionManager!!.requestExactAlarmPermission(call) - return - } - - // Permission granted - proceed with scheduling - val configJson = call.getObject("config") - val config = parseUserNotificationConfig(configJson) - - Log.i(TAG, "Scheduling user notification") - - CoroutineScope(Dispatchers.IO).launch { - try { - // Delegate to ScheduleHelper - val scheduleId = ScheduleHelper.scheduleUserNotification( - context, - getDatabase(), - config, - ::calculateNextRunTime - ) - - if (scheduleId != null) { - call.resolve() - } else { - call.reject("User notification scheduling failed") - } - } catch (e: Exception) { - Log.e(TAG, "Failed to schedule user notification", e) - call.reject("User notification scheduling failed: ${e.message}") - } - } - } catch (e: Exception) { - Log.e(TAG, "Schedule user notification error", e) - call.reject("User notification error: ${e.message}") - } - } - @PluginMethod fun scheduleDualNotification(call: PluginCall) { try { @@ -2776,8 +2725,8 @@ object ScheduleHelper { schedules.forEach { schedule -> val scheduleJson = scheduleToJson(schedule) - // Only check AlarmManager status for "notify" schedules with nextRunAt - if (schedule.kind == "notify" && schedule.nextRunAt != null) { + // Only check AlarmManager status for "notify"/"static_reminder" schedules with nextRunAt + if ((schedule.kind == "notify" || schedule.kind == "static_reminder") && schedule.nextRunAt != null) { val isScheduled = NotifyReceiver.isAlarmScheduled(context, scheduleId = schedule.id, triggerAtMillis = schedule.nextRunAt!!) scheduleJson.put("isActuallyScheduled", isScheduled) } else { @@ -2842,9 +2791,14 @@ object ScheduleHelper { // notifications at fire time. // Store schedule in database (include rollover interval for dev/testing; survives reboot) + // kind = "static_reminder" isolates this from the shared "notify" kind used by the + // dual/API-retrieval flow (generator #2) and legacy test-data injection, so the + // cleanup sweep in cleanupExistingNotificationSchedules() (which only scans + // kind="notify") can never touch or be confused by this row. See + // doc/progress/PLAN-simplify-static-and-refactor.md §3.1. val schedule = Schedule( id = scheduleId, - kind = "notify", + kind = "static_reminder", cron = config.schedule, clockTime = clockTime, enabled = true, @@ -2904,15 +2858,18 @@ object ScheduleHelper { } /** - * Blocking: first enabled notify schedule with rolloverIntervalMinutes > 0 (canonical for rollover chain). + * Blocking: first enabled static_reminder schedule with rolloverIntervalMinutes > 0 + * (canonical for rollover chain). * Used when the firing run has schedule_id = daily_rollover_* so we can still apply the interval. + * This whole function only exists to support that orphan-fallback path; it becomes dead + * code once §3.6 removes the daily_rollover_* fallback (see PLAN-simplify-static-and-refactor.md). */ @JvmStatic fun getCanonicalRolloverScheduleBlocking(context: Context): Schedule? { return kotlinx.coroutines.runBlocking { try { DailyNotificationDatabase.getDatabase(context).scheduleDao() - .getByKindAndEnabled("notify", true) + .getByKindAndEnabled("static_reminder", true) .firstOrNull { it.rolloverIntervalMinutes != null && it.rolloverIntervalMinutes > 0 } } catch (e: Exception) { Log.w("ScheduleHelper", "getCanonicalRolloverScheduleBlocking failed", e) @@ -3443,9 +3400,16 @@ object NotificationStatusHelper { */ suspend fun getNotificationStatus(database: DailyNotificationDatabase): JSObject { val schedules = database.scheduleDao().getAll() - val notifySchedules = schedules.filter { it.kind == "notify" && it.enabled } - - // Get last notification time from history + // Includes "static_reminder" alongside "notify" so status reporting (isEnabled, + // isScheduled, nextNotificationTime, etc.) still reflects the static reminder now + // that it's isolated to its own kind. See PLAN-simplify-static-and-refactor.md §3.1. + val notifySchedules = schedules.filter { + (it.kind == "notify" || it.kind == "static_reminder") && it.enabled + } + + // Get last notification time from history. History.kind is a fixed event-type tag + // written uniformly by NotifyReceiver.recordHistory() for any alarm fire (static + // reminder or dual-notify) — independent of Schedule.kind, so no change needed here. val history = database.historyDao().getRecent(100) // Get last 100 entries val lastNotification = history .filter { it.kind == "notify" && it.outcome == "success" } diff --git a/android/src/main/java/org/timesafari/dailynotification/DailyNotificationReceiver.java b/android/src/main/java/org/timesafari/dailynotification/DailyNotificationReceiver.java index 42070af..d1dc8d0 100644 --- a/android/src/main/java/org/timesafari/dailynotification/DailyNotificationReceiver.java +++ b/android/src/main/java/org/timesafari/dailynotification/DailyNotificationReceiver.java @@ -202,339 +202,14 @@ public class DailyNotificationReceiver extends BroadcastReceiver { } } - /** - * Handle notification intent - * - * @param context Application context - * @param intent Intent containing notification data - */ - private void handleNotificationIntent(Context context, Intent intent) { - try { - String notificationId = intent.getStringExtra(EXTRA_NOTIFICATION_ID); - - if (notificationId == null) { - Log.w(TAG, "Notification ID not found in intent"); - return; - } - - Log.d(TAG, "Processing notification: " + notificationId); - - // Get notification content from storage - DailyNotificationStorage storage = new DailyNotificationStorage(context); - NotificationContent content = storage.getNotificationContent(notificationId); - - if (content == null) { - Log.w(TAG, "Notification content not found: " + notificationId); - return; - } - - // Check if notification is ready to display - if (!content.isReadyToDisplay()) { - Log.d(TAG, "Notification not ready to display yet: " + notificationId); - return; - } - - // JIT Freshness Re-check (Soft TTL) - content = performJITFreshnessCheck(context, content); - - // Display the notification - displayNotification(context, content); - - // Schedule next notification if this is a recurring daily notification - scheduleNextNotification(context, content); - - Log.i(TAG, "Notification processed successfully: " + notificationId); - - } catch (Exception e) { - Log.e(TAG, "Error handling notification intent", e); - } - } - - /** - * Perform JIT (Just-In-Time) freshness re-check for notification content - * - * This implements a soft TTL mechanism that attempts to refresh stale content - * just before displaying the notification. If the refresh fails or content - * is not stale, the original content is returned. - * - * @param context Application context - * @param content Original notification content - * @return Updated content if refresh succeeded, original content otherwise - */ - private NotificationContent performJITFreshnessCheck(Context context, NotificationContent content) { - try { - // Check if content is stale (older than 6 hours for JIT check) - long currentTime = System.currentTimeMillis(); - long age = currentTime - content.getFetchedAt(); - long staleThreshold = 6 * 60 * 60 * 1000; // 6 hours in milliseconds - - if (age < staleThreshold) { - Log.d(TAG, "Content is fresh (age: " + (age / 1000 / 60) + " minutes), skipping JIT refresh"); - return content; - } - - Log.i(TAG, "Content is stale (age: " + (age / 1000 / 60) + " minutes), attempting JIT refresh"); - - // Attempt to fetch fresh content - DailyNotificationFetcher fetcher = new DailyNotificationFetcher(context, new DailyNotificationStorage(context)); - - // Attempt immediate fetch for fresh content - NotificationContent freshContent = fetcher.fetchContentImmediately(); - - if (freshContent != null && freshContent.getTitle() != null && !freshContent.getTitle().isEmpty()) { - Log.i(TAG, "JIT refresh succeeded, using fresh content"); - - // Update the original content with fresh data while preserving the original ID and scheduled time - String originalId = content.getId(); - long originalScheduledTime = content.getScheduledTime(); - - content.setTitle(freshContent.getTitle()); - content.setBody(freshContent.getBody()); - content.setSound(freshContent.isSound()); - content.setPriority(freshContent.getPriority()); - content.setUrl(freshContent.getUrl()); - content.setMediaUrl(freshContent.getMediaUrl()); - content.setScheduledTime(originalScheduledTime); // Preserve original scheduled time - // Note: fetchedAt remains unchanged to preserve original fetch time - - // Save updated content to storage - DailyNotificationStorage storage = new DailyNotificationStorage(context); - storage.saveNotificationContent(content); - - return content; - } else { - Log.w(TAG, "JIT refresh failed or returned empty content, using original content"); - return content; - } - - } catch (Exception e) { - Log.e(TAG, "Error during JIT freshness check", e); - return content; // Return original content on error - } - } - - /** - * Display the notification to the user - * - * @param context Application context - * @param content Notification content to display - */ - private void displayNotification(Context context, NotificationContent content) { - try { - Log.d(TAG, "Displaying notification: " + content.getId()); - - NotificationManager notificationManager = - (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE); - - if (notificationManager == null) { - Log.e(TAG, "NotificationManager not available"); - return; - } - - // Create notification builder - NotificationCompat.Builder builder = new NotificationCompat.Builder(context, CHANNEL_ID) - .setSmallIcon(android.R.drawable.ic_dialog_info) - .setContentTitle(content.getTitle()) - .setContentText(content.getBody()) - .setPriority(getNotificationPriority(content.getPriority())) - .setAutoCancel(true) - .setCategory(NotificationCompat.CATEGORY_REMINDER); - - // Add sound if enabled - if (content.isSound()) { - builder.setDefaults(NotificationCompat.DEFAULT_SOUND); - } - - // Add click action if URL is available - if (content.getUrl() != null && !content.getUrl().isEmpty()) { - Intent clickIntent = new Intent(Intent.ACTION_VIEW); - clickIntent.setData(android.net.Uri.parse(content.getUrl())); - - PendingIntent clickPendingIntent = PendingIntent.getActivity( - context, - content.getId().hashCode(), - clickIntent, - PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE - ); - - builder.setContentIntent(clickPendingIntent); - } - - // Add dismiss action - Intent dismissIntent = new Intent(context, DailyNotificationReceiver.class); - dismissIntent.setAction("org.timesafari.daily.DISMISS"); - dismissIntent.putExtra(EXTRA_NOTIFICATION_ID, content.getId()); - - PendingIntent dismissPendingIntent = PendingIntent.getBroadcast( - context, - content.getId().hashCode() + 1000, // Different request code - dismissIntent, - PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE - ); - - builder.addAction( - android.R.drawable.ic_menu_close_clear_cancel, - "Dismiss", - dismissPendingIntent - ); - - // Build and display notification - int notificationId = content.getId().hashCode(); - notificationManager.notify(notificationId, builder.build()); - - Log.i(TAG, "Notification displayed successfully: " + content.getId()); - - } catch (Exception e) { - Log.e(TAG, "Error displaying notification", e); - } - } - - /** - * Schedule the next occurrence of this daily notification - * - * Uses centralized NotifyReceiver.scheduleExactNotification() with ROLLOVER_ON_FIRE source - * to ensure idempotence and proper logging - * - * @param context Application context - * @param content Current notification content - */ - private void scheduleNextNotification(Context context, NotificationContent content) { - try { - Log.d(TAG, "Scheduling next notification for: " + content.getId()); - - // Extract scheduleId from notificationId pattern or use fallback - // Notification IDs are often "daily_${scheduleId}" - String scheduleId = null; - String cronExpression = null; - long nextScheduledTime = content.getScheduledTime() + (24 * 60 * 60 * 1000); - - // Try to extract scheduleId from notificationId (e.g., "daily_1764578136269") - String notificationId = content.getId(); - if (notificationId != null && notificationId.startsWith("daily_")) { - scheduleId = notificationId; // Use notificationId as scheduleId - } else { - scheduleId = "daily_rollover_" + System.currentTimeMillis(); - } - - // Calculate cron from current scheduled time (extract hour:minute) - try { - java.util.Calendar cal = java.util.Calendar.getInstance(); - cal.setTimeInMillis(content.getScheduledTime()); - int hour = cal.get(java.util.Calendar.HOUR_OF_DAY); - int minute = cal.get(java.util.Calendar.MINUTE); - cronExpression = String.format("%d %d * * *", minute, hour); - - // Recalculate next run time from cron (tomorrow at same time) - nextScheduledTime = calculateNextRunTimeFromCron(cronExpression); - } catch (Exception e) { - Log.w(TAG, "Failed to calculate cron from scheduled time, using default", e); - cronExpression = "0 9 * * *"; // Default to 9 AM - } - - // Create config for next notification - org.timesafari.dailynotification.UserNotificationConfig config = - new org.timesafari.dailynotification.UserNotificationConfig( - true, // enabled - cronExpression, - content.getTitle() != null ? content.getTitle() : "Daily Notification", - content.getBody(), - content.isSound(), - true, // vibration - content.getPriority() != null ? content.getPriority() : "normal" - ); - - // Use centralized scheduling function with ROLLOVER_ON_FIRE source - org.timesafari.dailynotification.NotifyReceiver.scheduleExactNotification( - context, - nextScheduledTime, - config, - false, // isStaticReminder - null, // reminderId - scheduleId, - org.timesafari.dailynotification.ScheduleSource.ROLLOVER_ON_FIRE, - false // skipPendingIntentIdempotence – rollover path does not skip - ); - - Log.i(TAG, "Next notification scheduled via centralized function: scheduleId=" + scheduleId); - - } catch (Exception e) { - Log.e(TAG, "Error scheduling next notification", e); - } - } - - /** - * Helper to convert HH:mm time to cron expression - */ - private String convertTimeToCron(String clockTime) { - try { - String[] parts = clockTime.split(":"); - if (parts.length == 2) { - int hour = Integer.parseInt(parts[0]); - int minute = Integer.parseInt(parts[1]); - return String.format("%d %d * * *", minute, hour); - } - } catch (Exception e) { - Log.w(TAG, "Failed to parse clockTime: " + clockTime, e); - } - return "0 9 * * *"; // Default to 9 AM - } - - /** - * Helper to calculate next run time from cron expression - */ - private long calculateNextRunTimeFromCron(String cron) { - try { - String[] parts = cron.trim().split("\\s+"); - if (parts.length >= 2) { - int minute = Integer.parseInt(parts[0]); - int hour = Integer.parseInt(parts[1]); - - java.util.Calendar calendar = java.util.Calendar.getInstance(); - long now = calendar.getTimeInMillis(); - - calendar.set(java.util.Calendar.HOUR_OF_DAY, hour); - calendar.set(java.util.Calendar.MINUTE, minute); - calendar.set(java.util.Calendar.SECOND, 0); - calendar.set(java.util.Calendar.MILLISECOND, 0); - - long nextRun = calendar.getTimeInMillis(); - if (nextRun <= now) { - calendar.add(java.util.Calendar.DAY_OF_YEAR, 1); - nextRun = calendar.getTimeInMillis(); - } - return nextRun; - } - } catch (Exception e) { - Log.w(TAG, "Failed to calculate next run time from cron: " + cron, e); - } - // Fallback: 24 hours from now - return System.currentTimeMillis() + (24 * 60 * 60 * 1000L); - } - - /** - * Get notification priority constant - * - * @param priority Priority string from content - * @return NotificationCompat priority constant - */ - private int getNotificationPriority(String priority) { - if (priority == null) { - return NotificationCompat.PRIORITY_DEFAULT; - } - - switch (priority.toLowerCase()) { - case "high": - return NotificationCompat.PRIORITY_HIGH; - case "low": - return NotificationCompat.PRIORITY_LOW; - case "min": - return NotificationCompat.PRIORITY_MIN; - case "max": - return NotificationCompat.PRIORITY_MAX; - default: - return NotificationCompat.PRIORITY_DEFAULT; - } - } + // handleNotificationIntent/performJITFreshnessCheck/displayNotification/ + // scheduleNextNotification/convertTimeToCron/calculateNextRunTimeFromCron/ + // getNotificationPriority were removed here (PLAN-simplify-static-and-refactor.md §3.6): + // onReceive() only ever calls enqueueNotificationWork()/enqueueDismissalWork() above, so + // this entire chain — including scheduleNextNotification's "daily_rollover_" + // orphan-id fallback (generator #4, the other source of stray uncancellable schedules) — + // was confirmed dead (zero callers) and fully superseded by DailyNotificationWorker.java's + // live equivalents. /** * Handle notification dismissal diff --git a/android/src/main/java/org/timesafari/dailynotification/DailyNotificationWorker.java b/android/src/main/java/org/timesafari/dailynotification/DailyNotificationWorker.java index fc30f43..f5be92d 100644 --- a/android/src/main/java/org/timesafari/dailynotification/DailyNotificationWorker.java +++ b/android/src/main/java/org/timesafari/dailynotification/DailyNotificationWorker.java @@ -572,7 +572,19 @@ public class DailyNotificationWorker extends Worker { Integer rolloverMinutes = null; if (logicalScheduleIdForRollover != null && !logicalScheduleIdForRollover.isEmpty()) { org.timesafari.dailynotification.Schedule s = org.timesafari.dailynotification.ScheduleHelper.getScheduleBlocking(getApplicationContext(), logicalScheduleIdForRollover); - if (s != null && s.getRolloverIntervalMinutes() != null && s.getRolloverIntervalMinutes() > 0) { + // Cancellation guard: if the user cancelled (row deleted, see cancelDailyReminder's + // deleteById) or disabled this schedule since the alarm fired, stop here rather than + // unconditionally re-arming the next occurrence. This closes the race described in + // PLAN-simplify-static-and-refactor.md §3.4 — without this check, scheduleNextNotification + // always re-armed regardless of cancellation state, which is why "cancel" didn't + // actually stop notifications that were already in-flight when cancelled. + if (s == null || !s.getEnabled()) { + Log.i(TAG, "DN|RESCHEDULE_SKIP_CANCELLED id=" + content.getId() + + " scheduleId=" + logicalScheduleIdForRollover + + " reason=" + (s == null ? "deleted" : "disabled")); + return; + } + if (s.getRolloverIntervalMinutes() != null && s.getRolloverIntervalMinutes() > 0) { rolloverMinutes = s.getRolloverIntervalMinutes(); Log.d(TAG, "DN|ROLLOVER_INTERVAL scheduleId=" + logicalScheduleIdForRollover + " minutes=" + rolloverMinutes); } @@ -612,9 +624,18 @@ public class DailyNotificationWorker extends Worker { String notificationId = content.getId(); String scheduleId = scheduleIdForRollover; if (scheduleId == null || scheduleId.isEmpty()) { - scheduleId = "daily_rollover_" + System.currentTimeMillis(); + // Previously fell back to a freshly-minted "daily_rollover_" id here, + // which created an orphan schedule row every time it triggered — itself a source + // of the "stray schedule that survives cancellation" bug (generator #4 in + // PLAN-simplify-static-and-refactor.md §2). Per §3.6, this branch should be + // unreachable for static reminders (NotifyReceiver.scheduleExactNotification + // always sets the schedule_id intent extra), so fail loudly instead of silently + // spawning an untrackable, uncancellable orphan alarm. + Log.e(TAG, "DN|RESCHEDULE_ABORT id=" + content.getId() + + " reason=no_resolvable_schedule_id - not re-arming to avoid creating an orphan schedule"); + return; } - + // When using rollover interval, next time already set; otherwise compute from cron (tomorrow same time) if (rolloverMinutes == null || rolloverMinutes <= 0) { try { diff --git a/android/src/main/java/org/timesafari/dailynotification/DatabaseSchema.kt b/android/src/main/java/org/timesafari/dailynotification/DatabaseSchema.kt index b77e797..17f8dab 100644 --- a/android/src/main/java/org/timesafari/dailynotification/DatabaseSchema.kt +++ b/android/src/main/java/org/timesafari/dailynotification/DatabaseSchema.kt @@ -87,7 +87,7 @@ data class History( NotificationDeliveryEntity::class, NotificationConfigEntity::class ], - version = 4, // 4: content_cache.cacheScope + version = 5, // 5: isolate static reminder rows into kind="static_reminder" (was "notify") exportSchema = false ) @TypeConverters(Converters::class) @@ -122,7 +122,7 @@ abstract class DailyNotificationDatabase : RoomDatabase() { DailyNotificationDatabase::class.java, DATABASE_NAME ) - .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4) + .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5) .addCallback(roomCallback) .build() INSTANCE = instance @@ -288,6 +288,31 @@ abstract class DailyNotificationDatabase : RoomDatabase() { ) } } + + /** + * Isolate the static reminder's row into its own "static_reminder" kind, away from the + * shared "notify" kind (see PLAN-simplify-static-and-refactor.md §3.1, §3.7). + * + * The static reminder's id is caller-supplied (TimeSafari uses "daily_timesafari_reminder"; + * the plugin defaults to "daily_notification" if none is given), so we can't match it by a + * fixed id. Instead, reclassify by elimination: any existing kind="notify" row that does + * NOT match one of the three other generators' known id prefixes + * (dual_notify_, notify_, daily_rollover_) must be the static reminder, since + * those are the only generators that ever wrote kind="notify" rows. + */ + val MIGRATION_4_5 = object : Migration(4, 5) { + override fun migrate(database: SupportSQLiteDatabase) { + database.execSQL( + """ + UPDATE schedules SET kind = 'static_reminder' + WHERE kind = 'notify' + AND id NOT LIKE 'dual_notify_%' + AND id NOT LIKE 'notify_%' + AND id NOT LIKE 'daily_rollover_%' + """.trimIndent() + ) + } + } } } diff --git a/android/src/main/java/org/timesafari/dailynotification/NotifyReceiver.kt b/android/src/main/java/org/timesafari/dailynotification/NotifyReceiver.kt index b188cb0..3faa461 100644 --- a/android/src/main/java/org/timesafari/dailynotification/NotifyReceiver.kt +++ b/android/src/main/java/org/timesafari/dailynotification/NotifyReceiver.kt @@ -435,7 +435,12 @@ class NotifyReceiver : BroadcastReceiver() { // nextRunAt with the rollover time and can leave the app's alarm in a bad state. if (scheduleToUpdate == null && !stableScheduleId.startsWith("daily_rollover_")) { val allSchedules = db.scheduleDao().getAll() - scheduleToUpdate = allSchedules.firstOrNull { it.kind == "notify" && it.enabled } + // Includes "static_reminder" so this fallback can find the real static + // reminder row (now isolated from "notify") instead of falling through to + // the "create new" branch below and spawning a duplicate row. + scheduleToUpdate = allSchedules.firstOrNull { + (it.kind == "notify" || it.kind == "static_reminder") && it.enabled + } } // Calculate cron expression from trigger time (HH:mm format) diff --git a/android/src/main/java/org/timesafari/dailynotification/ReactivationManager.kt b/android/src/main/java/org/timesafari/dailynotification/ReactivationManager.kt index 911e0d7..0d1aab6 100644 --- a/android/src/main/java/org/timesafari/dailynotification/ReactivationManager.kt +++ b/android/src/main/java/org/timesafari/dailynotification/ReactivationManager.kt @@ -117,26 +117,26 @@ class ReactivationManager(private val context: Context) { enabledSchedules.forEach { schedule -> try { when (schedule.kind) { - "notify" -> { + "notify", "static_reminder" -> { val nextRunTime = calculateNextRunTimeForSchedule(schedule, currentTime) - + if (nextRunTime < currentTime) { // Past alarm - mark as missed and schedule next occurrence markMissedNotificationForSchedule(schedule, nextRunTime, db) missedCount++ - + // Schedule next occurrence (use rollover interval if set, else 24h) val nextOccurrence = calculateNextOccurrenceForSchedule(schedule, nextRunTime, currentTime) rescheduleAlarmForBoot(context, schedule, nextOccurrence, db) rescheduledCount++ - + Log.i(TAG, "Marked missed notification: ${schedule.id}") Log.i(TAG, "Rescheduled alarm: ${schedule.id} for $nextOccurrence") } else { // Future alarm - reschedule immediately rescheduleAlarmForBoot(context, schedule, nextRunTime, db) rescheduledCount++ - + Log.i(TAG, "Rescheduled alarm: ${schedule.id} for $nextRunTime") } } @@ -627,7 +627,7 @@ class ReactivationManager(private val context: Context) { // Step 3: Verify and reschedule future alarms val schedules = try { db.scheduleDao().getEnabled() - .filter { it.kind == "notify" } + .filter { it.kind == "notify" || it.kind == "static_reminder" } } catch (e: Exception) { Log.e(TAG, "Failed to query schedules", e) emptyList() @@ -731,7 +731,7 @@ class ReactivationManager(private val context: Context) { } when (schedule.kind) { - "notify" -> { + "notify", "static_reminder" -> { val result = recoverNotifySchedule(schedule, currentTime, db) missedCount += result.missedCount rescheduledCount += result.rescheduledCount diff --git a/doc/_archive/PLAN-simplify-static-and-refactor.md b/doc/_archive/PLAN-simplify-static-and-refactor.md new file mode 100644 index 0000000..aef44cc --- /dev/null +++ b/doc/_archive/PLAN-simplify-static-and-refactor.md @@ -0,0 +1,461 @@ +# Plan: Simplify Static Reminder Path & Remove Dead Notify Generators + +**Purpose:** Fix the Android bug where cancelling a static daily reminder +doesn't stop it from firing, by giving static reminders their own isolated +schedule namespace — and remove now-unneeded legacy code along the way. + +**Status:** Ready for execution +**Date:** 2026-06-07 +**Platform:** Android only (the reported bug is Android-specific; iOS was not +investigated — see "iOS" section below before assuming parity) + +--- + +## 1. The bug this fixes + +In the TimeSafari PWA (`crowd-funder-for-time-pwa`), turning off the daily +reminder (`AccountViewView.vue` → `NativeNotificationService.cancelDailyNotification()` +→ plugin `cancelDailyReminder`) reports success, but the notification **keeps +firing every day**. + +`cancelDailyReminder` (`DailyNotificationPlugin.kt:754-779`) only: +1. Cancels the AlarmManager alarm for one specific `scheduleId` (the app's + stable id, `"daily_timesafari_reminder"`), and +2. Sets `enabled = false` for that one row in the `schedules` table. + +By contrast, `scheduleDailyNotification` (`DailyNotificationPlugin.kt:1096-1195`) +defensively sweeps the **entire** `schedules` table via +`ScheduleHelper.cleanupExistingNotificationSchedules()` (kills every +`kind == "notify"` row except the one being created) and cancels +prefetch/fetch WorkManager jobs via `cancelFetchRelatedWorkManagerJobs()` — +explicitly because, per its own comments, other code paths can spin up a +"second (UUID) alarm" that fires duplicate/stray notifications. + +So: if **any** stray `notify`-kind schedule exists with an ID other than +`"daily_timesafari_reminder"`, cancellation completely ignores it, and it +keeps re-arming itself and firing forever — because the rollover/rearm logic +(`DailyNotificationWorker.scheduleNextNotification`, +`DailyNotificationReceiver.scheduleNextNotification`) re-arms the *next* +occurrence unconditionally on every fire, **never checking the `enabled` flag**. + +--- + +## 2. Why the defensive sweeping exists (and why it can be deleted) + +The `schedules` table (`DatabaseSchema.kt:41-55`) is shared by *four* different +generators, distinguished only by a free-text `kind` column and by ID-prefix +*conventions* that are not enforced anywhere: + +| # | Generator | Schedule id pattern | Where | Used by TimeSafari? | +|---|---|---|---|---| +| 1 | **Static reminder** (the only thing TimeSafari uses) | caller-provided `id`, e.g. `"daily_timesafari_reminder"` | `scheduleDailyNotification`/`scheduleDailyReminder` → `ScheduleHelper.scheduleDailyNotification` (`DailyNotificationPlugin.kt:2743-2826`) | **Yes — the only path in use** | +| 2 | **Dual/API-retrieval** (planned, not yet used) | `dual_fetch_` / `dual_notify_` | `scheduleDualNotification` → `ScheduleHelper.scheduleDualNotification` (`DailyNotificationPlugin.kt:2890-2936`) + `DualScheduleNotifyScheduler.scheduleChainedNotifyAlarm` (`DualScheduleNotifyScheduler.kt:19`) | No (planned for future) | +| 3 | **Standalone user notification** (legacy, separate public API) | `notify_` | `scheduleUserNotification` plugin method (`DailyNotificationPlugin.kt:1364`) → `ScheduleHelper.scheduleUserNotification` (`DailyNotificationPlugin.kt:2949-2980`) | **No** — not called anywhere in `crowd-funder-for-time-pwa/src`; only referenced in the plugin's own docs/examples | +| 4 | **Orphan rollover fallback** (accidental, not a real feature) | `daily_rollover_` (regenerated fresh — never stabilizes) | Inline fallback inside `DailyNotificationReceiver.scheduleNextNotification` (`DailyNotificationReceiver.java:416`) and `DailyNotificationWorker.scheduleNextNotification` (`DailyNotificationWorker.java:614-616`), triggered when the rearm code can't resolve a stable schedule id from intent extras/content id | **No** — pure bug-patch byproduct | + +Generators #1 and #2 are real, intentional features (static reminders are live; +dual/API-retrieval is planned). Generators #3 and #4 are **not** part of either +of those — they are independent legacy/accidental code: + +- **#3 (`scheduleUserNotification`)** is its own standalone public plugin + method, parallel to (not called by) the dual flow. Confirmed unused by the + app — eliminable as dead surface area in its own right. +- **#4 (`daily_rollover_*` fallback)** is not tied to any feature; it's a + brittle fallback branch inside the *shared* rearm logic that can fire for + *any* notify schedule when ID-threading fails, and it spins off a fresh + unstable orphan id every time it triggers — i.e. it is itself a source of + the exact "stray schedule" problem that breaks cancellation. + +**Conclusion:** the cleanup-sweep machinery in `scheduleDailyNotification` +exists to paper over collisions between these four generators sharing one +table/AlarmManager namespace. If the static reminder (the only feature +TimeSafari actually uses today) is moved into its **own isolated `kind`** with +a single, always-known, stable id, none of that sweeping is needed for it — +scheduling and cancelling become trivial single-row operations, and the +"cancel doesn't stop it" bug is structurally impossible (there is nothing else +to collide with). + +--- + +## 3. Refactor plan + +### 3.1 Give static reminders their own `kind` + +- Change the `kind` value used for static reminders from `"notify"` to a new, + distinct value, e.g. `"static_reminder"`. +- Touch points (search for `kind = "notify"` and `isStaticReminder`): + - `ScheduleHelper.scheduleDailyNotification` (`DailyNotificationPlugin.kt:2784`, + `Schedule(... kind = "notify" ...)`) → change to `"static_reminder"`. + - Any `getByKind("notify")` / `getByKindAndEnabled("notify", ...)` calls that + are meant to operate on static reminders specifically (audit all call + sites at `DailyNotificationPlugin.kt:702, 727-728, 1661, 1722, 2716-2717, + 2852, 3079, 3283, 3288` — most of these are for the dual/fetch machinery + and should stay `"notify"`/`"fetch"`; only the ones that exist purely to + find/clean the static reminder's row need to move to `"static_reminder"`, + and after this refactor most of them become unnecessary for the static + path — see 3.2). +- This alone makes `cleanupExistingNotificationSchedules(excludeScheduleId)` + (which filters `getByKind("notify")`) blind to the static reminder row, + which is what we want — it should never need to "clean up" the one + legitimate static reminder. + +### 3.2 Simplify scheduling for static reminders + +Replace the sweep-and-recreate dance in `scheduleDailyNotification` +(`DailyNotificationPlugin.kt:1130-1190`) with a direct upsert-in-place: + +- Remove the call to `ScheduleHelper.cleanupExistingNotificationSchedules()` + (`DailyNotificationPlugin.kt:1140-1150`) for the static-reminder path — it + exists to prevent collisions with *other kinds*; with its own `kind`, the + static reminder can never collide. +- Remove the call to `ScheduleHelper.cancelFetchRelatedWorkManagerJobs()` + (`DailyNotificationPlugin.kt:1155-1158`) for the static-reminder path — + static reminders never enqueue prefetch/fetch work in the first place + (already explicitly skipped, see comment at `DailyNotificationPlugin.kt:2775-2779`), + so there is nothing to cancel. +- Keep `NotifyReceiver.cancelNotification(context, scheduleId)` + + `scheduleExactNotification(...)` (the actual alarm arm/rearm) — that part is + correct and necessary (it ensures "one alarm per id" via stable requestCode). + +### 3.3 Simplify cancellation for static reminders + +Rewrite `cancelDailyReminder` (`DailyNotificationPlugin.kt:754-779`) to be a +pure single-row operation, since with its own `kind` there is now guaranteed +to be at most one row to deal with: + +```kotlin +@PluginMethod +fun cancelDailyReminder(call: PluginCall) { + try { + val reminderId = call.getString("reminderId") + ?: call.getString("id") + ?: return call.reject("cancelDailyReminder: missing reminderId") + + NotifyReceiver.cancelNotification(context, scheduleId = reminderId) + + kotlinx.coroutines.runBlocking { + val db = getDatabase() + db.scheduleDao().deleteById(reminderId) // or setEnabled(false) — see note + db.notificationContentDao().deleteById(reminderId) // if such a method exists; else equivalent cleanup + } + call.resolve() + } catch (e: Exception) { + Log.e(TAG, "cancelDailyReminder failed", e) + call.reject("cancelDailyReminder failed: ${e.message}") + } +} +``` + +Note: decide `deleteById` vs `setEnabled(id, false)`: +- `deleteById` is cleaner (no stale rows ever accumulate) but means + `BootReceiver`'s `getEnabled()` sweep simply won't find the row — which is + correct (nothing to reschedule). +- `setEnabled(id, false)` keeps history/diagnostics but requires every rearm + path to check `enabled` before re-arming (see 3.4) — more moving parts. +- **Recommendation: `deleteById`.** It's the simplest correct option and + matches "cancel means gone," not "cancel means disabled-but-present." + +### 3.4 Make the rearm chain check for cancellation + +Even with `deleteById`, there is a narrow race: the alarm could already be +in-flight (fired, worker running) at the moment the user cancels. Today, +`scheduleNextNotification` in both `DailyNotificationWorker.java:547-667` and +`DailyNotificationReceiver.java:401-...` (the latter looks unreachable — see +3.6) **unconditionally** re-arms the next occurrence with no DB check. + +Add a guard at the top of `DailyNotificationWorker.scheduleNextNotification`: +look up the schedule row by id (`ScheduleHelper.getScheduleBlocking`); if it's +missing (deleted) or `enabled == false`, log and return without re-arming. +This closes the cancel race definitively and is a small, isolated change. + +### 3.5 Remove generator #3 — `scheduleUserNotification` + +**Confirmed unused in any executable path** — not called anywhere in +`crowd-funder-for-time-pwa/src`, and the only "usages" in the plugin repo +itself are inside doc/example files +(`test-apps/daily-notification-test/docs/VUE3_NOTIFICATION_IMPLEMENTATION_GUIDE.md`, +`examples/timesafari-integration-example.ts`) — not actual test-app source or +test suites. + +**Important nuance found on closer look:** those two doc/example usages both +call it as a one-shot, event-triggered notification — + +```typescript +await DailyNotification.scheduleUserNotification({ + schedule: 'immediate', + title: change.title, + body: change.message, + ... +}) +``` + +— i.e. "something happened in the app right now, show a notification." That's +a *third* semantic, distinct from both #1 (recurring static daily reminder) +and #2 (recurring fetch-then-notify cycle). **But it's also non-functional**: +the real implementation has no special case for `schedule: 'immediate'`. +`parseUserNotificationConfig` (`DailyNotificationPlugin.kt:2475-2484`) passes +the `schedule` string straight through, and +`ScheduleHelper.scheduleUserNotification` runs it through +`calculateNextRunTime` → `ScheduleCronUtils.calculateNextRunTimeMillis(schedule)` +(`DailyNotificationPlugin.kt:2487-2489`) — a **cron parser** that has no +concept of `"immediate"`. So even the one documented usage pattern would fail +or misbehave if anyone actually tried it. The docs describe a capability that +was never built; `scheduleUserNotification` itself is just a thin "schedule +one bare cron-based notification alarm" primitive that nothing exercises. + +**Recommendation: remove it, and do not resurrect the same capability under +the same API by patching in `'immediate'` support.** If "notify the user +immediately when the app detects an event" is still wanted later, design it +as its own clearly-named one-shot API (e.g. `notifyNow(options)` / +`showImmediateNotification(options)`) with **no `schedule`/cron field at +all** — just `title`/`body`/optional `triggerAtMillis` (default "now"), +routed through a "display once, never rearm" path. That keeps the three +concerns (recurring static reminder / recurring fetch+notify cycle / one-shot +event notification) distinguished by API *shape*, not by a magic string a +cron parser silently chokes on. + +**[DONE 2026-06-19]** Removed from Android only: +- `@PluginMethod fun scheduleUserNotification` (was `DailyNotificationPlugin.kt:1363-1411`) +- `ScheduleHelper.scheduleUserNotification` (was `DailyNotificationPlugin.kt:2899-2935`) +- No separate `methods.append(...)` registration existed — `@PluginMethod` annotation + is the only registration mechanism in this codebase, so nothing else to clean up there. +- Verified via `./gradlew :daily-notification-plugin:compileDebugKotlin` — compiles clean, + no other call sites referenced either removed symbol. + +**Scope correction — do NOT remove the TS/iOS layer.** `scheduleUserNotification` in +`src/definitions.ts:543` / `src/web.ts:231` is a **cross-platform** method signature, +and on iOS it is genuinely load-bearing: `DailyNotificationScheduleHelper.scheduleDualNotification` +(`ios/Plugin/DailyNotificationScheduleHelper.swift:98-107`) takes a +`scheduleUserNotification` closure that's bound to the real +`@objc func scheduleUserNotification` (`ios/Plugin/DailyNotificationPlugin.swift:379`) — +i.e. on iOS this *is* the dual flow's notification half, not a dead generator. +Android's version was a standalone, never-wired-up duplicate of that concept +(Android's actual dual flow uses `scheduleDualNotification`/`DualScheduleNotifyScheduler` +instead and never called `ScheduleHelper.scheduleUserNotification`) — so removing it +from Android only does not reduce iOS capability. Leave: +- `src/definitions.ts`/`src/web.ts` — keep the TS signature +- README section `#### scheduleUserNotification(config)` (lines ~295-306) — already + documents the correct cron-based shape (`schedule: string; // Cron expression`), + no `'immediate'` mentioned, nothing broken here +- iOS Swift implementation — untouched, see + [`PLAN-ios-static-reminder-fix.md`](./PLAN-ios-static-reminder-fix.md) §7 for why + +Still worth doing (low-priority, independent of this fix, not yet done): strip or +correct the broken `schedule: 'immediate'` example in +`test-apps/daily-notification-test/docs/VUE3_NOTIFICATION_IMPLEMENTATION_GUIDE.md:226-240` +and `examples/timesafari-integration-example.ts:101-109` — that pattern was never +implemented on either platform's actual cron parser, regardless of the Android +generator-#3 cleanup above. + +This eliminates the Android-only `notify_` id convention entirely — one less +thing that can collide in the shared `schedules` table — without touching the +cross-platform API surface or iOS's dual-flow capability. + +### 3.6 Remove generator #4 — `daily_rollover_*` fallback (and dead code around it) + +- In `DailyNotificationWorker.scheduleNextNotification` + (`DailyNotificationWorker.java:556-563, 613-616`): once static reminders + always carry a known, stable `schedule_id` intent extra (true today — + `NotifyReceiver.scheduleExactNotification` always sets it, + `NotifyReceiver.kt:297`), the `scheduleIdForRollover == null` branch should + be unreachable for the static-reminder path. After confirming (via logging + or a temporary assertion in a debug build) that it never fires for static + reminders, delete the `"daily_rollover_" + System.currentTimeMillis()` + fallback branches and the `getCanonicalRolloverScheduleBlocking` resolution + hack (`DailyNotificationWorker.java:564-571`, + `ScheduleHelper.getCanonicalRolloverScheduleBlocking` at + `DailyNotificationPlugin.kt:2844-2853`) that exists solely to paper over it. +- `DailyNotificationReceiver.handleNotificationIntent` / + `scheduleNextNotification` (`DailyNotificationReceiver.java:211-..., + 401-461`) appear to be **dead code** — `onReceive` only calls + `enqueueNotificationWork`/`enqueueDismissalWork` (`DailyNotificationReceiver.java:62-84`), + never `handleNotificationIntent`. Confirm with a repo-wide search for + callers, then delete the whole block (it duplicates, slightly differently, + what `DailyNotificationWorker` does — a second source of the same + `daily_rollover_*` bug pattern). + +### 3.7 Clean up DB after refactor + +- Add a one-time migration (or lazy cleanup on plugin init) that deletes any + pre-existing `notify_` and `daily_rollover_` rows from `schedules` + (and their `notification_content` counterparts) — these are exactly the + kind of orphan that causes the bug, and existing installs may already have + them sitting in the DB from earlier buggy runs. +- Re-point the existing `"daily_notification"` / app-supplied static reminder + row to `kind = "static_reminder"` in that same migration (`UPDATE schedules + SET kind = 'static_reminder' WHERE kind = 'notify' AND id = :appReminderId`, + or simpler: `WHERE id NOT LIKE 'dual_%' AND id NOT LIKE 'notify_%' AND id NOT + LIKE 'daily_rollover_%'` — i.e. whatever's left after excluding known + dual/legacy/orphan patterns is the static reminder). + +--- + +## 4. What NOT to touch (dual/API-retrieval flow — planned for future use) + +Per the user: the dual/API-retrieval flow ("device wakes by timer or push → +calls API → stores content → reschedules next check → OS later wakes app to +show stored content") is **not used today but is planned**. Leave generator #2 +and its supporting machinery alone: +- `scheduleDualNotification`, `ScheduleHelper.scheduleDualNotification` + (`DailyNotificationPlugin.kt:2890-2936`) +- `DualScheduleNotifyScheduler` (`DualScheduleNotifyScheduler.kt`) +- `FetchWorker`, `DualScheduleFetchRecovery`, content-cache (`cacheScope = + "dual"`), circuit breakers, etc. + +These keep using `kind = "notify"`/`"fetch"` and the existing sweep machinery +— which is fine, since that machinery's whole job is to keep *that* subsystem +internally consistent. The point of 3.1 is just to make sure the static +reminder's single row is no longer inside that sweep's blast radius (and vice +versa). + +--- + +## 5. iOS + +**Investigated — see [`PLAN-ios-static-reminder-fix.md`](./PLAN-ios-static-reminder-fix.md) +for the full analysis and fix plan. Summary:** + +iOS does **not** have an analogous shared-table-with-multiple-generators +design (no Room/SQLite `schedules` table, no AlarmManager, no rearm chain — +it uses OS-native `UNUserNotificationCenter` with `UNCalendarNotificationTrigger`, +which natively supports "repeat forever" and needs none of that machinery). + +But it has its own, structurally different bug with the same user-visible +symptom: the client's `scheduleDailyNotification`/`cancelDailyReminder` calls +land in **two unrelated, non-communicating subsystems** inside +`DailyNotificationPlugin.swift` — +- `scheduleDailyNotification` (:1392) registers under a fresh + `content.id = "daily_"` each call, with `repeats: false` + (likely **one-shot, not actually recurring** — see the linked plan's §4) +- `cancelDailyReminder` (:1174) only ever removes `"reminder_\(id)"`-shaped + identifiers — which the schedule path above never registers, so cancel is a + **silent no-op** that still reports success + +Meanwhile a *third*, already-correct subsystem +(`scheduleDailyReminder`/`cancelDailyReminder`/`updateDailyReminder`, :1088-1300) +sits unused right next to it — a single `repeats: true` calendar trigger under +a stable caller-supplied id, with matching cancel. The fix is to repoint +`scheduleDailyNotification` onto that existing correct mechanism rather than +to build Android-style sweep/rearm machinery iOS doesn't need. + +This is a fully independent fix (different files, different language, no +shared code) — see the linked plan's §5 for the side-by-side comparison. +Execute in either order, or in parallel. + +--- + +## 6. Verification plan + +1. **Unit/integration**: existing plugin test suite (`npm test`) plus new + tests asserting: + - Scheduling a static reminder creates exactly one `schedules` row with + `kind = "static_reminder"`. + - Cancelling deletes that row and cancels its `PendingIntent` + (`isAlarmScheduled` returns false afterward). + - No `notify_`/`dual_*`/`daily_rollover_*` rows are created or touched + by the static-reminder schedule/cancel/rearm cycle. + - **Note:** `grep`ing `android/src/test/java/.../*.kt` today shows **zero** + existing coverage of `cancelDailyReminder`, `scheduleNextNotification`, or + `cleanupExistingNotificationSchedules` — this bug had a test-coverage + blind spot. Add a regression test asserting "after `cancelDailyReminder`, + `scheduleDao().getByKind('notify')` (or `'static_reminder'` post-3.1) + returns zero rows" so this can't silently regress. + +2. **Manual on-device (Android emulator) — reproduce-then-verify sequence**: + + This environment already has `adb`, `emulator`, `gradle`, and two AVDs + (`Pixel_3a_API_34_extension_level_7_arm64-v8a`, `Pixel_7`) plus a mature + ADB-based toolkit in `test-apps/android-test-app/alarm-test-lib.sh` and + `doc/alarms/PHASE1-EMULATOR-TESTING.md` using exactly the techniques below. + `APP_ID = org.timesafari.dailynotification`. + + **Setup:** + ```bash + emulator -avd Pixel_7 -no-snapshot-load & + adb wait-for-device + # install a debug build of the test app or the TimeSafari PWA Android shell + ``` + + **Step A — schedule, then capture a baseline of native scheduler + DB state** + (don't trust "the call resolved" — inspect the actual OS/DB state, since + that's exactly where this bug hides): + ```bash + # AlarmManager state: + adb shell dumpsys alarm | grep -A 5 -i "org.timesafari.dailynotification" + + # DB state — the crucial one; reveals orphans dumpsys alone won't make obvious: + adb shell run-as org.timesafari.dailynotification sqlite3 \ + databases/daily_notification_plugin.db \ + "SELECT id, kind, enabled, nextRunAt FROM schedules;" + ``` + Record the count and exact `id`/`kind` values — this is the "before" baseline. + Pre-fix, expect to possibly see stray `daily_rollover_*`/`dual_notify_*`/ + `notify_` rows even before cancelling, which alone partially explains + the bug. + + **Step B — cancel via the app UI, then immediately re-run the same two + commands.** This is the actual test — not "did the app report success" + (it always does), but: does `dumpsys alarm` still show an entry, and does + `schedules` still contain *any* `kind='notify'`/`'static_reminder'` row + (enabled or not)? Pre-fix expectation per §1: the targeted row gets + `enabled=0` but isn't deleted, and/or a stray row survives untouched. + + **Step C — prove it actually still fires** (closes the loop — a surviving + row could theoretically be inert): + ```bash + # Time-travel the emulator clock forward (technique already documented at + # doc/alarms/PHASE1-EMULATOR-TESTING.md:412-418): + adb shell date -s "$(date -v+1d '+%Y-%m-%d %H:%M:%S')" + adb logcat -c + adb logcat -s DailyNotification:* NotifyReceiver:* DailyNotificationWorker:* \ + | grep -i "notification displayed\|scheduleNextNotification\|rearm" + + # Optional visual proof — capture and pull a screenshot: + adb shell screencap -p /sdcard/after-cancel.png && adb pull /sdcard/after-cancel.png + + # Reset the clock when done: + adb shell date -s "$(date '+%Y-%m-%d %H:%M:%S')" + ``` + Seeing "notification displayed successfully" / rearm log lines *after* + cancelling is the smoking gun. + + **Step D — re-run the full A→B→C sequence after applying the fix.** + Expected post-fix result: zero `dumpsys alarm` entries and zero `schedules` + rows immediately after cancel, and no log lines / no notification after the + time-jump. This A→B→C→(fix)→A→B→C before/after comparison *is* the proof + the refactor worked — anything less ("the call resolved without error") is + exactly the false confidence that let this bug ship originally. + + Also still worth doing once the structural fix is confirmed: + - Let it fire and rearm at least once more (or use + `rolloverIntervalMinutes` for fast iteration); confirm the row's id stays + stable across rearms + - Reboot the device after cancelling; confirm `BootReceiver` does not + resurrect it + +3. **Regression**: confirm the dual/API-retrieval test paths (if any exist in + `test-apps/`) still pass unchanged — this refactor must not touch generator + #2's behavior. + +4. **Fast deterministic checks to run alongside the above** (don't touch the + native bug directly, but catch regressions in surrounding plumbing and are + cheap to run on every change): + - `npm test` — Jest suite for the TS/JS plugin layer (web stub, type defs) + - `cd android && ./gradlew test` — Kotlin unit tests (currently has the + coverage gap noted in §6.1 above) + +--- + +## 7. Suggested execution order + +1. 3.5 (remove `scheduleUserNotification`) — fully independent, smallest blast + radius, immediate dead-code win. +2. 3.1 + 3.2 + 3.3 (new `kind`, simplified schedule/cancel) — the core fix. +3. 3.7 (DB migration/cleanup for existing installs). +4. 3.4 (`enabled` guard in rearm — closes the race). +5. 3.6 (remove `daily_rollover_*` fallback + dead `handleNotificationIntent` + code) — do this last, after 3.1-3.4 make it provably unreachable for the + static path; double check it isn't somehow load-bearing for the dual flow + first (it shouldn't be — dual ids never start with `"daily_"` and dual + rearming goes through `DualScheduleNotifyScheduler`, not this fallback — + but confirm before deleting).