Make the backup folder pick two-phase (closes #30)

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.

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<String, Any?> {
+        // 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<Pair<String, String>> {
@@ -116,11 +159,11 @@ object BackupStore {
         children(context, tree).firstOrNull { it.second == name }
             ?.let { DocumentsContract.buildDocumentUriUsingTree(tree, it.first) }
 
-    fun list(context: Context): List<String> =
-        children(context, requireFolder(context)).map { it.second }
+    fun list(context: Context, pending: Boolean = false): List<String> =
+        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)
     }

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<Boolean>("pending") == true)
+                    }
 
                     "readBackup" -> io(result) {
-                        BackupStore.read(this, call.argument<String>("name")!!)
+                        BackupStore.read(
+                            this,
+                            call.argument<String>("name")!!,
+                            pending = call.argument<Boolean>("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) }
         }
     }

lib/services/backup_bridge.dart

@@ -9,24 +9,35 @@ class BackupBridge {
         await _channel.invokeMapMethod<String, Object?>('backupStatus') ?? const {},
       );
 
-  /// Opens the system folder picker. Null if the user backed out.
-  static Future<BackupFolderStatus?> 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.
+  static Future<String?> pickFolder() async {
     final raw = await _channel.invokeMapMethod<String, Object?>('pickBackupFolder');
-    return raw == null ? null : BackupFolderStatus.fromMap(raw);
+    return raw?['folderName'] as String?;
   }
 
+  /// Makes the pending folder the backup folder.
+  static Future<void> confirmFolder() => _channel.invokeMethod('confirmBackupFolder');
+
+  /// Drops the pending folder without making it active; releases its grant.
+  static Future<void> discardFolder() => _channel.invokeMethod('discardBackupFolder');
+
   /// Whether Android's Google backup may copy app data. Off unless the user turns it on.
   static Future<bool> googleBackupEnabled() async =>
       await _channel.invokeMethod<bool>('googleBackupEnabled') ?? false;
 
   static Future<void> setGoogleBackupEnabled(bool enabled) =>
       _channel.invokeMethod('setGoogleBackupEnabled', {'enabled': enabled});
 
-  static Future<List<String>> list() async =>
-      await _channel.invokeListMethod<String>('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<String>> list({bool pending = false}) async =>
+      await _channel.invokeListMethod<String>('listBackups', {'pending': pending}) ??
+      const [];
 
-  static Future<String> read(String name) async =>
-      (await _channel.invokeMethod<String>('readBackup', {'name': name}))!;
+  static Future<String> read(String name, {bool pending = false}) async =>
+      (await _channel.invokeMethod<String>('readBackup', {'name': name, 'pending': pending}))!;
 
   static Future<void> write(String name, String content) =>
       _channel.invokeMethod('writeBackup', {'name': name, 'content': content});

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<Backup> snapshot({DateTime? now}) async => Backup(
         exportedAt: now ?? DateTime.now(),
         medications: await _db.medications(includeDeleted: true),
@@ -55,9 +49,6 @@ class BackupService {
 
   Future<void> _running = Future.value();
 
-  /// Completes once every backup already queued has finished.
-  Future<void> get backupsIdle => _running;
-
   Future<BackupFolderStatus> _backUp({DateTime? now}) async {
     final status = await BackupBridge.status();
     if (!status.enabled) return status;
@@ -90,7 +81,6 @@ class BackupService {
   /// refresh.
   Future<BackupFolderStatus?> backUpQuietly() async {
     try {
-      if (holds > 0) return await BackupBridge.status();
       return await backUp();
     } on PlatformException catch (e) {
       lastError = e.message ?? e.code;

lib/ui/backup_page.dart

@@ -62,32 +62,43 @@ class _BackupPageState extends State<BackupPage> {
   }
 
   Future<void> _chooseFolder() => _run(() async {
-        _service.holds++;
+        // The picked folder stays *pending* until an explicit confirm, so any exit
+        // 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 {
-          // 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
           // 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 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;
+            });
+            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 +149,7 @@ class _BackupPageState extends State<BackupPage> {
 
   /// Confirms, then replaces everything. Always confirmed: a restore overwrites every
   /// medication and all history currently on the phone. True once restored.
-  Future<bool> _restore(Backup backup) async {
+  Future<bool> _restore(Backup backup, {Future<void> Function()? onReplaced}) async {
     final taken = backup.doses.where((d) => d.status == DoseStatus.taken).length;
     if (!mounted) return false;
     final confirmed = await showDialog<bool>(
@@ -166,6 +177,7 @@ class _BackupPageState extends State<BackupPage> {
     if (confirmed != true) return false;
 
     await widget.scheduler.restore(backup);
+    await onReplaced?.call();
     await _service.backUpQuietly();
     _toast('Restored ${backup.medications.length} medication(s). Alarms re-armed.');
     return true;