From fb64f7ea6a670914503b985ebb85f895115e545b Mon Sep 17 00:00:00 2001 From: SusieeTheFemboy <96988330+SusieeTheLinuxUser@users.noreply.github.com> Date: Sat, 7 Nov 2026 03:11:59 +0100 Subject: [PATCH 1/2] Make the backup folder pick two-phase (closes #30) A picked folder is now stored as pending, not active, until the "restore or keep?" question is answered. Keep, or a finished restore, confirms it; any other exit (back out, page closed, error) discards it and releases its grant. Automatic backups only ever use the confirmed folder, so the Dart-side holds/backupsIdle guards are gone. On the restore path the folder is confirmed right before the data is replaced, so the restore's own backup lands in the new folder and the old folder's file for today keeps the phone's pre-restore data. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 13 +++ .../dev/susiee/dosekeeper/BackupStore.kt | 79 ++++++++++++++----- .../dev/susiee/dosekeeper/MainActivity.kt | 29 +++++-- lib/services/backup_bridge.dart | 25 ++++-- lib/services/backup_service.dart | 10 --- lib/ui/backup_page.dart | 42 ++++++---- 6 files changed, 141 insertions(+), 57 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cda2831..1f34c19 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,10 @@ feature. Do not trade it away for elegance or convenience. ## Current state (2026-11-04) +2026-11-07: the backup folder pick is **two-phase** (issue #30) -- a picked folder is +pending until the "restore or keep?" question is answered, so an unanswered pick can no +longer redirect automatic backups. See the Backup & restore entry under "Implemented". + 2026-11-07: deleting a medication is a **soft delete** (`medications.deleted`, DB v3) -- a hard DELETE cascaded through `doses` and destroyed the whole history the confirm dialog promised to keep; the row stays, flagged `deleted = 1` + `active = 0`, hidden from user-facing lists. @@ -372,6 +376,15 @@ the user's phone. `master` is branch-protected — see "Git workflow" below. that already has backups offers to restore first; restore is always confirmed, is one SQLite transaction, re-ids everything, and first drains native outcomes and cancels every armed alarm (incl. snoozes) so no orphaned alarm survives it. + - Two-phase folder pick (2026-11-07, issue #30): a picked folder is stored *pending* + (pref `pending_tree_uri`), not active, until Dart's "restore or keep?" is answered + and `confirmBackupFolder` moves it to `tree_uri`; every other exit calls + `discardBackupFolder`. Before this, the picker made the folder active immediately, + and leaving the dialog unanswered (killed app, closed page, unreadable folder) could + let the next automatic backup write into a folder the user never confirmed. + Automatic backups only ever use `tree_uri`; a leftover pending folder is discarded + on the next pick. The old Dart-side `holds`/`backupsIdle` guards are gone -- the + pending folder is invisible to automatic backups natively, so no hold is needed. - Today screen warns "Backups are off" (no folder / permission gone) and "Automatic backup failed: …", same pattern as the alarm-path warnings. - Restore verified on hardware 2026-09-24; see "Earlier on 2026-09-24" for what's left. diff --git a/android/app/src/main/kotlin/dev/susiee/dosekeeper/BackupStore.kt b/android/app/src/main/kotlin/dev/susiee/dosekeeper/BackupStore.kt index 856676b..22db795 100644 --- a/android/app/src/main/kotlin/dev/susiee/dosekeeper/BackupStore.kt +++ b/android/app/src/main/kotlin/dev/susiee/dosekeeper/BackupStore.kt @@ -23,9 +23,13 @@ import java.io.FileNotFoundException object BackupStore { private const val PREFS = "dosekeeper_backup" private const val KEY_TREE_URI = "tree_uri" + private const val KEY_PENDING_TREE_URI = "pending_tree_uri" private const val KEY_LAST_BACKUP_AT = "last_backup_at" private const val MIME_JSON = "application/json" + private const val GRANT_FLAGS = + Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION + private fun prefs(context: Context) = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) @@ -43,24 +47,61 @@ object BackupStore { if (enabled) marker.createNewFile() else marker.delete() } - /** Takes a lasting grant on [tree] (survives reboots) and makes it the backup folder. */ - fun setFolder(context: Context, tree: Uri) { - val flags = Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION - context.contentResolver.takePersistableUriPermission(tree, flags) + /** + * Takes a lasting grant on [tree] (survives reboots) and keeps it as the *pending* + * folder: not the backup folder until [confirmPendingFolder]. Two-phase because the + * picker answers before Dart has asked "restore or keep" -- if answering made the + * folder active right away, leaving the dialog unanswered (app killed, page closed) + * would point automatic backups at a folder the user never confirmed. + * A pending folder left over from a killed app is discarded on the next pick. + */ + fun setPendingFolder(context: Context, tree: Uri): Map { + // Grant first: if the tree is the same as a leftover pending, the discard must + // not release the grant we are about to rely on. + context.contentResolver.takePersistableUriPermission(tree, GRANT_FLAGS) + val leftover = prefs(context).getString(KEY_PENDING_TREE_URI, null) + prefs(context).edit().putString(KEY_PENDING_TREE_URI, tree.toString()).apply() + if (leftover != null && leftover != tree.toString()) { + releaseGrant(context, Uri.parse(leftover)) + } + return mapOf("folderName" to folderName(context, tree)) + } + + /** Makes the pending folder the backup folder. No-op without one. */ + fun confirmPendingFolder(context: Context) { + val pending = prefs(context).getString(KEY_PENDING_TREE_URI, null) ?: return // Let go of the previous folder's grant: Android caps how many an app may hold. - folderUri(context)?.takeIf { it != tree }?.let { - runCatching { context.contentResolver.releasePersistableUriPermission(it, flags) } + val previous = prefs(context).getString(KEY_TREE_URI, null) + prefs(context).edit() + .putString(KEY_TREE_URI, pending) + .remove(KEY_PENDING_TREE_URI) + .apply() + if (previous != null && previous != pending) { + releaseGrant(context, Uri.parse(previous)) } - prefs(context).edit().putString(KEY_TREE_URI, tree.toString()).apply() + } + + /** Drops the pending folder without making it active. */ + fun discardPendingFolder(context: Context) { + val pending = prefs(context).getString(KEY_PENDING_TREE_URI, null) ?: return + prefs(context).edit().remove(KEY_PENDING_TREE_URI).apply() + releaseGrant(context, Uri.parse(pending)) + } + + /** Never releases the active folder's grant: pending may point at the same folder. */ + private fun releaseGrant(context: Context, uri: Uri) { + if (uri.toString() == prefs(context).getString(KEY_TREE_URI, null)) return + runCatching { context.contentResolver.releasePersistableUriPermission(uri, GRANT_FLAGS) } } /** - * The backup folder, or null if none was chosen or the grant is gone (revoked in - * settings, or the folder was deleted) -- the caller must treat that as "backups - * are off", not as silently succeeding. + * The backup folder (or the pending one with [pending]), or null if none was chosen + * or the grant is gone (revoked in settings, or the folder was deleted) -- the caller + * must treat that as "backups are off", not as silently succeeding. */ - fun folderUri(context: Context): Uri? { - val stored = prefs(context).getString(KEY_TREE_URI, null) ?: return null + fun folderUri(context: Context, pending: Boolean = false): Uri? { + val key = if (pending) KEY_PENDING_TREE_URI else KEY_TREE_URI + val stored = prefs(context).getString(key, null) ?: return null val uri = Uri.parse(stored) val granted = context.contentResolver.persistedUriPermissions.any { it.uri == uri && it.isWritePermission && it.isReadPermission @@ -88,8 +129,10 @@ object BackupStore { return name ?: tree.lastPathSegment ?: "Backup folder" } - private fun requireFolder(context: Context): Uri = - folderUri(context) ?: throw FileNotFoundException("No backup folder chosen") + private fun requireFolder(context: Context, pending: Boolean = false): Uri = + folderUri(context, pending) ?: throw FileNotFoundException( + if (pending) "No picked folder waiting to be confirmed" else "No backup folder chosen", + ) /** (documentId, displayName) of every file directly inside the folder. */ private fun children(context: Context, tree: Uri): List> { @@ -116,11 +159,11 @@ object BackupStore { children(context, tree).firstOrNull { it.second == name } ?.let { DocumentsContract.buildDocumentUriUsingTree(tree, it.first) } - fun list(context: Context): List = - children(context, requireFolder(context)).map { it.second } + fun list(context: Context, pending: Boolean = false): List = + children(context, requireFolder(context, pending)).map { it.second } - fun read(context: Context, name: String): String { - val tree = requireFolder(context) + fun read(context: Context, name: String, pending: Boolean = false): String { + val tree = requireFolder(context, pending) val doc = find(context, tree, name) ?: throw FileNotFoundException(name) return readUri(context, doc) } diff --git a/android/app/src/main/kotlin/dev/susiee/dosekeeper/MainActivity.kt b/android/app/src/main/kotlin/dev/susiee/dosekeeper/MainActivity.kt index f434367..f00ef2d 100644 --- a/android/app/src/main/kotlin/dev/susiee/dosekeeper/MainActivity.kt +++ b/android/app/src/main/kotlin/dev/susiee/dosekeeper/MainActivity.kt @@ -241,10 +241,28 @@ class MainActivity : FlutterActivity() { REQ_PICK_BACKUP_FOLDER, ) - "listBackups" -> io(result) { BackupStore.list(this) } + "listBackups" -> io(result) { + BackupStore.list(this, pending = call.argument("pending") == true) + } "readBackup" -> io(result) { - BackupStore.read(this, call.argument("name")!!) + BackupStore.read( + this, + call.argument("name")!!, + pending = call.argument("pending") == true, + ) + } + + /** The user answered "keep" or restored: the pending folder becomes active. */ + "confirmBackupFolder" -> io(result) { + BackupStore.confirmPendingFolder(this) + true + } + + /** Any other exit from the folder pick: the pending folder goes away. */ + "discardBackupFolder" -> io(result) { + BackupStore.discardPendingFolder(this) + true } "writeBackup" -> io(result) { @@ -311,10 +329,9 @@ class MainActivity : FlutterActivity() { return } when (requestCode) { - REQ_PICK_BACKUP_FOLDER -> io(result) { - BackupStore.setFolder(this, uri) - BackupStore.status(this) - } + // Pending, not active: Dart still has to ask "restore or keep?" and only a + // confirmBackupFolder makes this the backup folder (see BackupStore). + REQ_PICK_BACKUP_FOLDER -> io(result) { BackupStore.setPendingFolder(this, uri) } REQ_PICK_BACKUP_FILE -> io(result) { BackupStore.readUri(this, uri) } } } diff --git a/lib/services/backup_bridge.dart b/lib/services/backup_bridge.dart index 838716e..d7ca2d2 100644 --- a/lib/services/backup_bridge.dart +++ b/lib/services/backup_bridge.dart @@ -9,12 +9,20 @@ class BackupBridge { await _channel.invokeMapMethod('backupStatus') ?? const {}, ); - /// Opens the system folder picker. Null if the user backed out. - static Future pickFolder() async { + /// Opens the system folder picker. The picked folder is *pending*, not active: it + /// only becomes the backup folder via [confirmFolder]. Returns the pending folder's + /// name, or null if the user backed out (then nothing is pending). + static Future pickFolder() async { final raw = await _channel.invokeMapMethod('pickBackupFolder'); - return raw == null ? null : BackupFolderStatus.fromMap(raw); + return raw?['folderName'] as String?; } + /// Makes the pending folder the backup folder. + static Future confirmFolder() => _channel.invokeMethod('confirmBackupFolder'); + + /// Drops the pending folder without making it active; releases its grant. + static Future discardFolder() => _channel.invokeMethod('discardBackupFolder'); + /// Whether Android's Google backup may copy app data. Off unless the user turns it on. static Future googleBackupEnabled() async => await _channel.invokeMethod('googleBackupEnabled') ?? false; @@ -22,11 +30,14 @@ class BackupBridge { static Future setGoogleBackupEnabled(bool enabled) => _channel.invokeMethod('setGoogleBackupEnabled', {'enabled': enabled}); - static Future> list() async => - await _channel.invokeListMethod('listBackups') ?? const []; + /// Lists the backup folder, or the pending one with [pending] (a folder just picked + /// that has not been confirmed yet). + static Future> list({bool pending = false}) async => + await _channel.invokeListMethod('listBackups', {'pending': pending}) ?? + const []; - static Future read(String name) async => - (await _channel.invokeMethod('readBackup', {'name': name}))!; + static Future read(String name, {bool pending = false}) async => + (await _channel.invokeMethod('readBackup', {'name': name, 'pending': pending}))!; static Future write(String name, String content) => _channel.invokeMethod('writeBackup', {'name': name, 'content': content}); diff --git a/lib/services/backup_service.dart b/lib/services/backup_service.dart index 3f166bc..456cce5 100644 --- a/lib/services/backup_service.dart +++ b/lib/services/backup_service.dart @@ -27,12 +27,6 @@ class BackupService { /// none, for the same reason an alarm that quietly doesn't ring is. String? lastError; - /// Folder choices in progress. While any is open, its resume must not write today's - /// file before the backup page has asked whether to restore what is already there. - /// A count, not a flag: a second choice started while the first one is still - /// waiting must keep the hold when the first one ends. - int holds = 0; - Future snapshot({DateTime? now}) async => Backup( exportedAt: now ?? DateTime.now(), medications: await _db.medications(includeDeleted: true), @@ -55,9 +49,6 @@ class BackupService { Future _running = Future.value(); - /// Completes once every backup already queued has finished. - Future get backupsIdle => _running; - Future _backUp({DateTime? now}) async { final status = await BackupBridge.status(); if (!status.enabled) return status; @@ -90,7 +81,6 @@ class BackupService { /// refresh. Future backUpQuietly() async { try { - if (holds > 0) return await BackupBridge.status(); return await backUp(); } on PlatformException catch (e) { lastError = e.message ?? e.code; diff --git a/lib/ui/backup_page.dart b/lib/ui/backup_page.dart index 7490a22..6e0747b 100644 --- a/lib/ui/backup_page.dart +++ b/lib/ui/backup_page.dart @@ -62,32 +62,41 @@ class _BackupPageState extends State { } Future _chooseFolder() => _run(() async { - _service.holds++; - try { - // A backup queued just before the hold must not land in the new folder. - await _service.backupsIdle; - if (!mounted) return; // left the page while waiting - final picked = await BackupBridge.pickFolder(); - if (picked == null) return; // backed out + final name = await BackupBridge.pickFolder(); + if (name == null) return; // backed out of the picker; nothing is pending + // The picked folder stays *pending* until an explicit confirm, so any exit + // without an answer (page closed, error) leaves the old folder active and + // automatic backups untouched. The finally discards it. + var confirmed = false; + try { // Picking a folder that already holds backups is what someone does after a - // reinstall. Offer the restore *before* the first automatic backup can - // overwrite today's file with whatever is on the phone now. - final existing = Backup.newestFirst(await BackupBridge.list()); + // reinstall. Offer the restore *before* the first backup can overwrite + // today's file with whatever is on the phone now. + final existing = + Backup.newestFirst(await BackupBridge.list(pending: true)); // Asked until answered: cancelling the restore's own confirmation comes back // here instead of counting as "keep". while (existing.isNotEmpty) { if (!mounted) return; // nobody to ask: don't write over what is there final restore = await _askRestoreFromFolder(existing.first); if (restore != true) break; // keep this phone's data - if (await _restore(Backup.decode(await BackupBridge.read(existing.first)))) { - return; - } + final backup = + Backup.decode(await BackupBridge.read(existing.first, pending: true)); + // Confirmed before the restore runs, so its backup lands in this folder and + // the old folder's file for today keeps this phone's pre-restore data. + final restored = await _restore(backup, beforeReplace: () async { + await BackupBridge.confirmFolder(); + confirmed = true; + }); + if (restored) return; } + await BackupBridge.confirmFolder(); + confirmed = true; await _service.backUp(); - _toast('Backups will be saved to ${picked.folderName}'); + _toast('Backups will be saved to $name'); } finally { - _service.holds--; + if (!confirmed) await BackupBridge.discardFolder(); } }); @@ -138,7 +147,7 @@ class _BackupPageState extends State { /// Confirms, then replaces everything. Always confirmed: a restore overwrites every /// medication and all history currently on the phone. True once restored. - Future _restore(Backup backup) async { + Future _restore(Backup backup, {Future Function()? beforeReplace}) async { final taken = backup.doses.where((d) => d.status == DoseStatus.taken).length; if (!mounted) return false; final confirmed = await showDialog( @@ -164,6 +173,7 @@ class _BackupPageState extends State { ), ); if (confirmed != true) return false; + await beforeReplace?.call(); await widget.scheduler.restore(backup); await _service.backUpQuietly(); From b32841947fda1f4f190bf90dc08435b54a7fc7da Mon Sep 17 00:00:00 2001 From: SusieeTheFemboy <96988330+SusieeTheLinuxUser@users.noreply.github.com> Date: Sat, 7 Nov 2026 03:17:05 +0100 Subject: [PATCH 2/2] Discard the pending folder when the pick fails; confirm only after a restore replaced the data - pickFolder now runs inside the try, so a throw from the native pick still reaches the finally that discards the pending folder. - The folder is confirmed after scheduler.restore succeeds (and before the restore's own backup), so a failed restore leaves it unconfirmed and no automatic backup can overwrite the file being restored. Co-Authored-By: Claude Opus 5.5 --- lib/services/backup_bridge.dart | 2 +- lib/ui/backup_page.dart | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/services/backup_bridge.dart b/lib/services/backup_bridge.dart index d7ca2d2..404e5e7 100644 --- a/lib/services/backup_bridge.dart +++ b/lib/services/backup_bridge.dart @@ -11,7 +11,7 @@ class BackupBridge { /// Opens the system folder picker. The picked folder is *pending*, not active: it /// only becomes the backup folder via [confirmFolder]. Returns the pending folder's - /// name, or null if the user backed out (then nothing is pending). + /// name, or null if the user backed out. static Future pickFolder() async { final raw = await _channel.invokeMapMethod('pickBackupFolder'); return raw?['folderName'] as String?; diff --git a/lib/ui/backup_page.dart b/lib/ui/backup_page.dart index 6e0747b..1944a6d 100644 --- a/lib/ui/backup_page.dart +++ b/lib/ui/backup_page.dart @@ -62,14 +62,14 @@ class _BackupPageState extends State { } Future _chooseFolder() => _run(() async { - final name = await BackupBridge.pickFolder(); - if (name == null) return; // backed out of the picker; nothing is pending - // The picked folder stays *pending* until an explicit confirm, so any exit - // without an answer (page closed, error) leaves the old folder active and - // automatic backups untouched. The finally discards it. + // without an answer (backed out, page closed, error -- the pick itself + // included) leaves the old folder active and automatic backups untouched. + // The finally discards it. var confirmed = false; try { + final name = await BackupBridge.pickFolder(); + if (name == null) return; // backed out of the picker // Picking a folder that already holds backups is what someone does after a // reinstall. Offer the restore *before* the first backup can overwrite // today's file with whatever is on the phone now. @@ -83,9 +83,11 @@ class _BackupPageState extends State { if (restore != true) break; // keep this phone's data final backup = Backup.decode(await BackupBridge.read(existing.first, pending: true)); - // Confirmed before the restore runs, so its backup lands in this folder and - // the old folder's file for today keeps this phone's pre-restore data. - final restored = await _restore(backup, beforeReplace: () async { + // Confirmed once the data is replaced but before the restore's own backup: + // that backup lands in this folder, the old folder's file for today keeps + // the pre-restore data, and a failed restore leaves this folder unconfirmed + // so no automatic backup can overwrite the file being restored. + final restored = await _restore(backup, onReplaced: () async { await BackupBridge.confirmFolder(); confirmed = true; }); @@ -147,7 +149,7 @@ class _BackupPageState extends State { /// Confirms, then replaces everything. Always confirmed: a restore overwrites every /// medication and all history currently on the phone. True once restored. - Future _restore(Backup backup, {Future Function()? beforeReplace}) async { + Future _restore(Backup backup, {Future Function()? onReplaced}) async { final taken = backup.doses.where((d) => d.status == DoseStatus.taken).length; if (!mounted) return false; final confirmed = await showDialog( @@ -173,9 +175,9 @@ class _BackupPageState extends State { ), ); if (confirmed != true) return false; - await beforeReplace?.call(); await widget.scheduler.restore(backup); + await onReplaced?.call(); await _service.backUpQuietly(); _toast('Restored ${backup.medications.length} medication(s). Alarms re-armed.'); return true;