diff --git a/CLAUDE.md b/CLAUDE.md index eb00270..ca90a1e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,16 +54,50 @@ log (`_sync_merge.merge_logs`: union by `id`, tombstone wins, legacy regardless of origin, rebuilds the food bank, then pushes this device's own merged log back up. +**The phone's periodic tick must sync unconditionally.** `checkAndNotify` +(`due_slot_check.dart`) once gated its sync on `due.isNotEmpty` — "only pull +when a slot looks overdue". That is a trap: logging every meal promptly on the +phone means nothing is ever due, so the phone never published, and the PC's +gate kept locking for slots that *were* already logged (the only way through +being to spam "nothing"). It publishes first, then computes due slots from the +merged result, so a meal pulled from another device suppresses a nag instead of +triggering one. Do not re-gate the sync on the notification's condition. + +**Anything running in a WorkManager background isolate must call +`initSyncDeviceId()` itself.** A fresh isolate has its own static state, so +without it `currentSyncDeviceId` silently falls back to the compile-time +`'phone'`/`'desktop'` role constant and the tick publishes to +`devices/phone/` instead of the persisted uuid. Because `sync_state` holds a +single `pushedRev` with no notion of which identity wrote it, the next +*foreground* push then sees `unchanged` and skips writing the uuid directory +entirely. `background_sync_service.dart` does this alongside its other +singleton re-inits; `background_sync_service_test.dart` guards it. + +**Catch `RemoteSyncError`, never `GitHubSyncError`, on any sync path.** +`FirebaseSyncError`/`FirebaseAuthError` are *siblings* of `GitHubSyncError` +under `RemoteSyncError`, not subclasses. Catching the GitHub type alone lets +every per-request failure of the *primary* backend escape — which made +`pull_shared_log` (the gate's "Fetch from sync" button, documented to fail +closed with a reason string) raise a traceback instead. +`ConfigError` is the one to watch: it subclasses `Exception` **directly**, so +`RemoteSyncError` does not cover it either. Anything that builds a Firebase +client must translate it (`_client_for_run` raises `SyncError`), or it escapes +every caller's catch tuple by a different route. + Re-signing on every merge (not just phone-origin entries) is the non-negotiable step: `_entry_is_valid()` drops any unsigned entry once a machine has the shared HMAC key, and the phone never holds that key, so skipping the re-sign would silently lose every phone-logged meal on the very next read. -Requires a one-time manual setup `install.sh` does **not** automate: create a -fine-grained GitHub PAT scoped to `syncs`'s contents (read/write), -then save it to `~/.config/diet_guard/sync_token`, mode 600. Until that file -exists, every sync tick is a harmless no-op that logs `sync not configured`. +Needs **one** backend configured, not both. Either `~/.config/crdt-sync/` +(Firebase, now the primary) or a fine-grained GitHub PAT scoped to `syncs`'s +contents (read/write) at `~/.config/diet_guard/sync_token`, mode 600 — +a one-time manual setup `install.sh` does **not** automate. A +Firebase-configured machine with no PAT syncs against Firebase alone +(`_client_for_run`); only when *neither* exists is the tick a harmless no-op +that logs `sync not configured`. The PAT used to be required unconditionally, +which left a Firebase-only PC unable to sync at all. **Both halves of the food bank sync.** The derived bank (`food_bank.json`) is still rebuilt locally from the merged log diff --git a/app/lib/screens/log_meal_screen.dart b/app/lib/screens/log_meal_screen.dart index 74ac5e6..c78e85d 100644 --- a/app/lib/screens/log_meal_screen.dart +++ b/app/lib/screens/log_meal_screen.dart @@ -24,12 +24,14 @@ import 'package:diet_guard_app/services/firebase_backend.dart'; import 'package:diet_guard_app/services/foodbank_service.dart'; import 'package:diet_guard_app/services/github_client_factory.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; +import 'package:diet_guard_app/services/sync_health.dart'; import 'package:diet_guard_app/services/sync_service.dart'; import 'package:diet_guard_app/services/sync_settings.dart'; import 'package:diet_guard_app/ui/theme.dart'; import 'package:diet_guard_app/widgets/autocomplete_suggestion_list.dart'; import 'package:diet_guard_app/widgets/macro_input_row.dart'; import 'package:diet_guard_app/widgets/slot_selector_row.dart'; +import 'package:diet_guard_app/widgets/sync_health_banner.dart'; import 'package:diet_guard_app/widgets/today_progress_card.dart'; import 'package:flutter/material.dart'; import 'package:http/http.dart' as http; @@ -62,6 +64,9 @@ class _LogMealScreenState extends State /// Single-flight guard so a launch sync and a lifecycle sync never overlap. bool _autoSyncing = false; + /// Latest sync health, driving the "not syncing" banner. Null until read. + SyncHealthStatus? _syncHealth; + @override void initState() { super.initState(); @@ -80,6 +85,9 @@ class _LogMealScreenState extends State _selectedSlot = slotForLog(DateTime.now()); unawaited(_refreshSlots()); unawaited(_onDescChanged()); + // Read health before the first sync finishes, so a device that stalled in + // a previous session says so immediately rather than only after a tick. + unawaited(_refreshSyncHealth()); unawaited(_autoSync()); } @@ -125,18 +133,31 @@ class _LogMealScreenState extends State // gating on it alone silently skips every sync on a device connected // only to Firebase -- and once the mirror is retired that is every // device. Same fix workout_app's push() already carries. - if (!settings.isConfigured && await openFirebase() == null) return; + if (!settings.isConfigured && await openFirebase() == null) { + // Never a silent return: an unconfigured device looked exactly like a + // healthy one with nothing to send, which is how meals sat unpublished + // for days. Recorded so the banner can say so. + log( + 'diet_guard auto-sync: no backend configured; nothing to push', + level: 900, + ); + await SyncHealth.recordUnconfigured(); + if (mounted) await _refreshSyncHealth(); + return; + } final client = createGitHubClient( settings, httpClient: widget.httpClient, ); try { await runSync(await syncBackend(client)); + await SyncHealth.recordSuccess(); } finally { client.close(); } if (!mounted) return; await _refreshSlots(); + await _refreshSyncHealth(); } on Object catch (error, stackTrace) { // Best-effort, but never silent: this swallowed the reason a desktop // install could not publish at all, which looked identical to "nothing @@ -148,11 +169,20 @@ class _LogMealScreenState extends State error: error, stackTrace: stackTrace, ); + await SyncHealth.recordFailure(); + if (mounted) await _refreshSyncHealth(); } finally { _autoSyncing = false; } } + /// Re-reads sync health so the banner reflects the tick that just ran. + Future _refreshSyncHealth() async { + final status = await SyncHealth.read(); + if (!mounted) return; + setState(() => _syncHealth = status); + } + /// Queues the platform's offline push backstop so a meal logged while /// offline still uploads on reconnect. The in-process [_autoSync] covers /// the online case; this is the backstop (a no-op on web, which has no @@ -359,6 +389,7 @@ class _LogMealScreenState extends State child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ + SyncHealthBanner(status: _syncHealth), SlotSelectorRow( now: DateTime.now(), loggedSlots: _loggedSlots, diff --git a/app/lib/screens/settings_screen.dart b/app/lib/screens/settings_screen.dart index 80f170e..adb75e7 100644 --- a/app/lib/screens/settings_screen.dart +++ b/app/lib/screens/settings_screen.dart @@ -15,6 +15,7 @@ import 'package:diet_guard_app/services/app_settings_service.dart'; import 'package:diet_guard_app/services/firebase_backend.dart'; import 'package:diet_guard_app/services/github_client_factory.dart'; import 'package:diet_guard_app/services/github_device_auth.dart'; +import 'package:diet_guard_app/services/sync_health.dart'; import 'package:diet_guard_app/services/sync_service.dart'; import 'package:diet_guard_app/services/sync_settings.dart'; import 'package:diet_guard_app/ui/theme.dart'; @@ -211,8 +212,10 @@ class _SettingsScreenState extends State { final client = createGitHubClient(settings, httpClient: widget.httpClient); try { await runSync(await syncBackend(client)); + await SyncHealth.recordSuccess(); _showMessage('Connected and synced.'); } on Exception catch (e) { + await SyncHealth.recordFailure(); _showMessage('Connected, but sync failed: $e'); } finally { client.close(); @@ -251,8 +254,13 @@ class _SettingsScreenState extends State { final client = createGitHubClient(settings, httpClient: widget.httpClient); try { await runSync(await syncBackend(client)); + // Clears any stored failure: this is the button a user reaches + // *because* the banner told them syncing had stopped, so a successful + // run here must dismiss the warning it caused. + await SyncHealth.recordSuccess(); _showMessage('Synced.'); } on Exception catch (e) { + await SyncHealth.recordFailure(); _showMessage('Sync failed: $e'); } finally { client.close(); diff --git a/app/lib/services/background_sync_service.dart b/app/lib/services/background_sync_service.dart index 29b1dae..e307dc8 100644 --- a/app/lib/services/background_sync_service.dart +++ b/app/lib/services/background_sync_service.dart @@ -12,6 +12,8 @@ import 'package:diet_guard_app/services/firebase_backend.dart'; import 'package:diet_guard_app/services/foodbank_service.dart'; import 'package:diet_guard_app/services/github_client_factory.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; +import 'package:diet_guard_app/services/sync_device_id.dart'; +import 'package:diet_guard_app/services/sync_health.dart'; import 'package:diet_guard_app/services/sync_service.dart'; import 'package:diet_guard_app/services/sync_settings.dart'; import 'package:http/http.dart' as http; @@ -34,6 +36,15 @@ const String syncPushTaskName = 'diet_guard.sync_push'; /// budget, and applies a merge winner via `AppSettingsService.instance`, /// which throws if the singleton was never initialised in this isolate. Future backgroundSyncPush({http.Client? httpClient}) async { + // Must run here too, and before anything stamps an HLC: a fresh isolate has + // its own static state, so without this `currentSyncDeviceId` falls back to + // the compile-time role constant ('phone') and this tick publishes to + // `devices/phone/` instead of the persisted uuid. That splits the device + // across two directories, and because `sync_state` holds a single + // `pushedRev` with no notion of which identity wrote it, the next + // *foreground* push then sees `unchanged` and skips writing the uuid + // directory entirely. + await initSyncDeviceId(); await LogStorageService.init(); await FoodBankService.init(); await AppSettingsService.init(); @@ -59,11 +70,13 @@ Future backgroundSyncPush({http.Client? httpClient}) async { 'diet_guard background sync: no backend configured; nothing to push', level: 900, ); + await SyncHealth.recordUnconfigured(); return true; // nothing to push; don't retry } final client = createGitHubClient(settings, httpClient: httpClient); try { await runSync(await syncBackend(client)); + await SyncHealth.recordSuccess(); return true; } on Exception catch (error, stackTrace) { // Never silent: this is the only signal that a background push failed. @@ -73,6 +86,7 @@ Future backgroundSyncPush({http.Client? httpClient}) async { error: error, stackTrace: stackTrace, ); + await SyncHealth.recordFailure(); return false; // offline / transient error -> retry with backoff } finally { client.close(); diff --git a/app/lib/services/due_slot_check.dart b/app/lib/services/due_slot_check.dart index 13b3e40..7468808 100644 --- a/app/lib/services/due_slot_check.dart +++ b/app/lib/services/due_slot_check.dart @@ -18,12 +18,20 @@ import 'package:http/http.dart' as http; /// Reads the local log, computes today's due-but-unlogged slots as of /// [now] (defaults to the real clock), and syncs notifications to match. /// -/// Pulls from GitHub *only* when a slot looks due, then re-checks -- the -/// local log alone cannot see a meal logged on the PC, so without this the -/// phone re-fires "you haven't logged your 12:00 meal" every background tick -/// until the app is next opened. Deliberately mirrors `_cli_gate._should_lock` -/// on the Python side: cheap local check first, network only when it would -/// change the answer, then decide on the merged result. +/// Syncs on every tick it is asked to ([pullWhenDue], the WorkManager +/// caller's default), then computes due slots from the merged result. +/// +/// Deliberately NOT gated on "a slot looks due": for the periodic caller this +/// is the phone's only publish path, mirroring the PC's +/// `diet-guard-sync.timer`. +/// Gating the sync on `due.isNotEmpty` meant that promptly logging every meal +/// on the phone left nothing ever due, so the phone never pushed and the PC +/// kept locking for slots that were already logged -- the exact failure this +/// check exists to prevent. A notification job that happens to sync is not a +/// sync timer. +/// +/// Order matters: the sync runs *before* [missingSlots], so a meal pulled from +/// another device suppresses a nag rather than triggering one. /// /// Set [pullWhenDue] false when the caller has just written to the local log /// itself (the log screen). There the local copy is by definition the freshest @@ -41,17 +49,16 @@ Future checkAndNotify({ }) async { await LogStorageService.init(); final at = now ?? DateTime.now(); - var due = missingSlots( - at, - await LogStorageService.instance.loggedSlotsToday(), - ); - if (pullWhenDue && due.isNotEmpty) { + if (pullWhenDue) { // Reuses backgroundSyncPush's settings/isConfigured/try-catch guard rather // than reimplementing it: that path is already proven to work from a // WorkManager isolate, including the token read and singleton init. await backgroundSyncPush(httpClient: httpClient); - due = missingSlots(at, await LogStorageService.instance.loggedSlotsToday()); } + final due = missingSlots( + at, + await LogStorageService.instance.loggedSlotsToday(), + ); await _reconcileNotifications(due); } diff --git a/app/lib/services/firebase_backend.dart b/app/lib/services/firebase_backend.dart index 327b011..165cd78 100644 --- a/app/lib/services/firebase_backend.dart +++ b/app/lib/services/firebase_backend.dart @@ -99,11 +99,21 @@ Future openFirebase() async { store: credentialStore(), password: account.password, ); - } on Object { + } on Object catch (error, stackTrace) { // Unreachable keystore or a rejected sign-in must not fail the tick: the // app keeps syncing over GitHub, exactly as before the cutover. // Broader than Exception because an uninitialised platform binding // raises a FlutterError, which is an Error. + // + // Never silent, for the same reason loadAccount() above says why: a + // signed-in-but-failing device looked exactly like an unconfigured one, + // which is how the phone went days without publishing a single meal. + log( + 'openFirebase failed; falling back to the GitHub mirror', + level: 1000, + error: error, + stackTrace: stackTrace, + ); return null; } } diff --git a/app/lib/services/sync_health.dart b/app/lib/services/sync_health.dart new file mode 100644 index 0000000..b3eed4d --- /dev/null +++ b/app/lib/services/sync_health.dart @@ -0,0 +1,164 @@ +/// Whether this device is actually publishing, and when it last managed to. +/// +/// Exists because the failure it reports was invisible for days: the phone +/// held meals nobody else could see, the PC's gate kept locking for slots that +/// *were* logged, and every screen looked completely normal. A sync that +/// silently stops must not be indistinguishable from a sync with nothing to +/// say, so the outcome of every tick is recorded here and surfaced by +/// [SyncHealthBanner] on the log screen. +/// +/// This is the *only* signal that reaches the user on a release build. The +/// `log()` calls at each failure site go to `dart:developer`, which surfaces +/// through the VM service and **not** logcat on a release APK -- verified on +/// device: zero Dart-side lines appear under `adb logcat` even from paths that +/// definitely ran. So the logging is for a debug session; the banner is what +/// makes a stalled sync noticeable in real use. Do not "simplify" this away in +/// favour of logging alone. +/// +/// Backed by SharedPreferences rather than a [DocumentStore] because a +/// WorkManager background isolate writes it too: prefs need no per-isolate +/// init, and a lost health stamp is cosmetic, never user data. +library; + +import 'package:shared_preferences/shared_preferences.dart'; + +/// How stale the last success may get before the banner appears. +/// +/// Comfortably longer than the ~15 min periodic tick, so one missed or +/// battery-deferred tick stays quiet and only a real stall shows up. +const Duration kSyncStaleAfter = Duration(hours: 6); + +/// Why the last tick did not publish, or null when it did. +enum SyncFailureKind { + /// No backend at all: neither a Firebase account nor a GitHub token. + unconfigured, + + /// A backend is configured but the attempt threw (offline, auth, remote). + failed, +} + +/// Records and reports the outcome of sync attempts. +abstract final class SyncHealth { + static const _kLastSuccess = 'sync.health.lastSuccessMs'; + static const _kFailureKind = 'sync.health.failureKind'; + + /// Marks a tick that actually reached the remote. + static Future recordSuccess() => _withPrefs((prefs) async { + await prefs.setInt(_kLastSuccess, DateTime.now().millisecondsSinceEpoch); + await prefs.remove(_kFailureKind); + }); + + /// Marks a tick that could not publish because nothing is set up. + static Future recordUnconfigured() => + _recordFailure(SyncFailureKind.unconfigured); + + /// Marks a tick that had a backend but failed to publish. + static Future recordFailure() => + _recordFailure(SyncFailureKind.failed); + + static Future _recordFailure(SyncFailureKind kind) => + _withPrefs((prefs) => prefs.setString(_kFailureKind, kind.name)); + + /// The current health, for the banner. + /// + /// Reports "healthy" when prefs are unreachable: a banner that cannot read + /// its own state must not accuse a working device of being stalled. + static Future read({DateTime? now}) async { + final at = now ?? DateTime.now(); + SharedPreferences prefs; + try { + prefs = await SharedPreferences.getInstance(); + } on Object { + return SyncHealthStatus(lastSuccess: null, failureKind: null, now: at); + } + final millis = prefs.getInt(_kLastSuccess); + final stored = prefs.getString(_kFailureKind); + return SyncHealthStatus( + lastSuccess: millis == null + ? null + : DateTime.fromMillisecondsSinceEpoch(millis), + failureKind: SyncFailureKind.values + .where((value) => value.name == stored) + .firstOrNull, + now: at, + ); + } + + /// Forgets all recorded health. Test-only. + static Future resetForTest() => _withPrefs((prefs) async { + await prefs.remove(_kLastSuccess); + await prefs.remove(_kFailureKind); + }); + + /// Runs [action] against prefs, ignoring an unreachable platform channel. + /// + /// Health is diagnostics about syncing, never user data, so it must never + /// take down the caller. Under `flutter test` the shared_preferences channel + /// is unmocked by default and throws [MissingPluginException] -- the same + /// reason [runSync]'s callers treat that as equivalent to being offline. + static Future _withPrefs( + Future Function(SharedPreferences prefs) action, + ) async { + try { + await action(await SharedPreferences.getInstance()); + } on Object { + // Losing a health stamp costs a banner, never a meal. + } + } +} + +/// A snapshot of whether this device is publishing. +class SyncHealthStatus { + /// Creates a snapshot evaluated as of [now]. + const SyncHealthStatus({ + required this.lastSuccess, + required this.failureKind, + required this.now, + }); + + /// When this device last published successfully, or null if it never has. + final DateTime? lastSuccess; + + /// Why the most recent attempt did not publish, or null when it did. + final SyncFailureKind? failureKind; + + /// The moment this snapshot was evaluated against. + final DateTime now; + + /// True when the user should be told that syncing has stopped. + /// + /// A device that has never synced is *not* flagged: a fresh install with no + /// account yet is a normal state, and nagging there would train the banner + /// to be ignored. Only a device that used to work, or one that explicitly + /// failed, is worth interrupting for. + bool get isStalled { + if (failureKind == SyncFailureKind.failed) return true; + final last = lastSuccess; + if (last == null) return false; + return now.difference(last) > kSyncStaleAfter; + } + + /// One line explaining the state, or null when there is nothing to say. + String? get message { + if (!isStalled) return null; + if (failureKind == SyncFailureKind.unconfigured) { + return 'Not syncing: no account connected. Meals stay on this device.'; + } + final since = lastSuccess == null ? null : now.difference(lastSuccess!); + // An elapsed time is only worth showing once it has actually elapsed. + // A sync that failed moments after a success rendered "Not syncing since + // 0h ago", which reads like a bug in the banner rather than a report + // about the sync. + if (since == null || since < const Duration(hours: 1)) { + return 'Not syncing. Meals stay on this device.'; + } + return 'Not syncing since ${_ago(since)}. Meals stay on this device.'; + } + + static String _ago(Duration since) { + if (since.inDays >= 1) { + return '${since.inDays}d ago'; + } + return '${since.inHours}h ago'; + } +} diff --git a/app/lib/widgets/sync_health_banner.dart b/app/lib/widgets/sync_health_banner.dart new file mode 100644 index 0000000..bd2efef --- /dev/null +++ b/app/lib/widgets/sync_health_banner.dart @@ -0,0 +1,57 @@ +/// The "not syncing" banner for the log screen. +library; + +import 'package:diet_guard_app/services/sync_health.dart'; +import 'package:diet_guard_app/ui/theme.dart'; +import 'package:flutter/material.dart'; + +/// Warns, in place, that meals logged here are not reaching other devices. +/// +/// Renders nothing at all unless [SyncHealthStatus.isStalled], so a healthy +/// device never pays for it. This is deliberately on the log screen rather +/// than buried in Settings: the whole failure mode being guarded against is +/// one that stayed invisible while the user kept logging meals that went +/// nowhere. +class SyncHealthBanner extends StatelessWidget { + /// Creates a [SyncHealthBanner] for [status]. + const SyncHealthBanner({required this.status, super.key}); + + /// The health to render, or null before the first read. + final SyncHealthStatus? status; + + @override + Widget build(BuildContext context) { + final message = status?.message; + if (message == null) return const SizedBox.shrink(); + final theme = Theme.of(context); + // Null-safe lookup, matching slot_selector_row/day_status_calendar: a + // widget test that pumps a bare MaterialApp has no theme extension, and + // `!` there is a crash rather than a missing colour. + final warning = + (theme.extension() ?? AppStatusColors.dark).warning; + return Container( + margin: const EdgeInsets.only(bottom: 12), + padding: const EdgeInsets.symmetric(horizontal: 12, vertical: 8), + decoration: BoxDecoration( + // A tinted fill plus a border, no shadow: the unified design system + // allows only one depth technique per surface, and shadows are out + // entirely in dark UI. + color: warning.withValues(alpha: 0.12), + border: Border.all(color: warning.withValues(alpha: 0.5)), + borderRadius: BorderRadius.circular(8), + ), + child: Row( + children: [ + Icon(Icons.cloud_off, size: 18, color: warning), + const SizedBox(width: 8), + Expanded( + child: Text( + message, + style: theme.textTheme.bodySmall?.copyWith(color: warning), + ), + ), + ], + ), + ); + } +} diff --git a/app/pubspec.yaml b/app/pubspec.yaml index d3f4023..d57d4f6 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -2,7 +2,7 @@ name: diet_guard_app description: "Companion phone app for diet_guard: log meals on the go." publish_to: 'none' -version: 1.0.3+1 +version: 1.0.4+1 environment: sdk: ^3.12.2 diff --git a/app/test/screens/settings_screen_test.dart b/app/test/screens/settings_screen_test.dart index 23a99e6..6cad498 100644 --- a/app/test/screens/settings_screen_test.dart +++ b/app/test/screens/settings_screen_test.dart @@ -9,6 +9,7 @@ import 'package:diet_guard_app/services/budget_history_service.dart'; import 'package:diet_guard_app/services/firebase_backend.dart'; import 'package:diet_guard_app/services/foodbank_service.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; +import 'package:diet_guard_app/services/sync_health.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -199,6 +200,40 @@ void main() { }); }); + testWidgets('Sync now clears a stored failure so the banner lifts', ( + tester, + ) async { + // "Sync now" is the button a user reaches *because* the log screen's + // banner told them syncing had stopped. If a successful run here left + // the recorded failure in place, the recovery action would not dismiss + // the warning it caused, and the banner would keep accusing a device + // that is now publishing fine. + await SyncHealth.recordFailure(); + expect((await SyncHealth.read()).failureKind, SyncFailureKind.failed); + + final mock = MockClient((req) async { + if (req.method == 'PUT') return http.Response('{}', 200); + if (req.method == 'GET' && req.url.pathSegments.length == 3) { + return http.Response('{}', 200); + } + return http.Response('', 404); + }); + await tester.runAsync(() async { + await tester.pumpWidget( + MaterialApp(home: SettingsScreen(httpClient: mock)), + ); + await settle(tester); + + await tester.tap(find.widgetWithText(ElevatedButton, 'Sync now')); + await settle(tester); + + expect(find.text('Synced.'), findsOneWidget); + final status = await SyncHealth.read(); + expect(status.failureKind, isNull); + expect(status.isStalled, isFalse); + }); + }); + testWidgets('Test connection reports a network exception', (tester) async { final mock = MockClient((_) async => throw const FormatException('no net')); await tester.runAsync(() async { diff --git a/app/test/services/background_sync_service_test.dart b/app/test/services/background_sync_service_test.dart index fb0c6aa..d6e185f 100644 --- a/app/test/services/background_sync_service_test.dart +++ b/app/test/services/background_sync_service_test.dart @@ -11,6 +11,7 @@ import 'package:diet_guard_app/services/budget_history_service.dart'; import 'package:diet_guard_app/services/background_sync_service.dart'; import 'package:diet_guard_app/services/foodbank_service.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; +import 'package:diet_guard_app/services/sync_device_id.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; @@ -87,6 +88,41 @@ void main() { expect(puts, 5); }); + test('pushes under the persisted uuid, not the legacy role id', () async { + // Regression guard. A WorkManager background isolate has its own static state, + // so without initSyncDeviceId() here `currentSyncDeviceId` falls back to + // the compile-time 'phone' constant and this tick writes to + // `devices/phone/`. That splits the device across two directories, and + // since sync_state holds a single `pushedRev` with no notion of which + // identity wrote it, the next *foreground* push then sees `unchanged` + // and skips writing the uuid directory at all. + const uuid = '77e39198-ff83-479b-a905-b0cd68b66094'; + SharedPreferences.setMockInitialValues({ + 'sync.owner': 'o', + 'sync.repo': 'r', + 'crdt.nodeId': uuid, + }); + resetSyncDeviceIdForTest(); + installFakeSecureStorage(initial: {'sync.token': 't'}); + final pushedPaths = []; + final mock = MockClient((req) async { + if (req.method == 'PUT') { + pushedPaths.add(req.url.path); + return http.Response('{}', 200); + } + if (req.method == 'GET' && req.url.pathSegments.length == 3) { + return http.Response('{}', 200); + } + return http.Response('', 404); + }); + + await backgroundSyncPush(httpClient: mock); + + expect(pushedPaths, isNotEmpty); + expect(pushedPaths.every((path) => path.contains(uuid)), isTrue); + expect(pushedPaths.any((path) => path.contains('/devices/phone/')), isFalse); + }); + test('reports failure (retry) when the push errors', () async { SharedPreferences.setMockInitialValues({ 'sync.owner': 'o', diff --git a/app/test/services/due_slot_check_test.dart b/app/test/services/due_slot_check_test.dart index 9e78cb2..3abada6 100644 --- a/app/test/services/due_slot_check_test.dart +++ b/app/test/services/due_slot_check_test.dart @@ -108,7 +108,7 @@ void main() { ); }); - test('does not pull when nothing is due', () async { + test('still syncs when nothing is due', () async { await LogStorageService.instance.logMeal('breakfast', _manual, slot: 8); SharedPreferences.setMockInitialValues({ 'sync.owner': 'o', @@ -123,9 +123,12 @@ void main() { await checkAndNotify(now: DateTime(2026, 1, 1, 8), httpClient: mock); - // Every due slot was already satisfied locally, so the network is never - // touched -- the check stays cheap on the overwhelmingly common tick. - expect(requests, 0); + // Regression guard. This asserted `requests == 0` -- "nothing due, so + // never touch the network" -- which is precisely the bug: logging every + // meal promptly on the phone left nothing ever due, so this periodic task + // never published and the PC kept locking for slots that WERE logged. + // This is the phone's only unconditional publish path; it must always run. + expect(requests, greaterThan(0)); }); test('pullWhenDue false skips the network even with a slot due', () async { diff --git a/app/test/services/sync_health_test.dart b/app/test/services/sync_health_test.dart new file mode 100644 index 0000000..b8239a6 --- /dev/null +++ b/app/test/services/sync_health_test.dart @@ -0,0 +1,134 @@ +import 'package:diet_guard_app/services/sync_health.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +void main() { + setUp(() => SharedPreferences.setMockInitialValues({})); + + group('SyncHealthStatus.isStalled', () { + test('a device that has never synced is not flagged', () { + // A fresh install with no account yet is normal, not a fault; nagging + // there would train the banner to be ignored. + final fresh = SyncHealthStatus( + lastSuccess: null, + failureKind: null, + now: DateTime(2026, 8, 10), + ); + expect(fresh.isStalled, isFalse); + expect(fresh.message, isNull); + }); + + test('a recent success is healthy', () { + final now = DateTime(2026, 8, 10, 12); + final status = SyncHealthStatus( + lastSuccess: now.subtract(const Duration(minutes: 30)), + failureKind: null, + now: now, + ); + expect(status.isStalled, isFalse); + }); + + test('a success older than the staleness window is flagged', () { + final now = DateTime(2026, 8, 10, 12); + final status = SyncHealthStatus( + lastSuccess: now.subtract(kSyncStaleAfter + const Duration(hours: 1)), + failureKind: null, + now: now, + ); + expect(status.isStalled, isTrue); + expect(status.message, contains('Not syncing since')); + }); + + test('an outright failure is flagged even when recent', () { + final now = DateTime(2026, 8, 10, 12); + final status = SyncHealthStatus( + lastSuccess: now.subtract(const Duration(minutes: 1)), + failureKind: SyncFailureKind.failed, + now: now, + ); + expect(status.isStalled, isTrue); + }); + + test('an unconfigured device names the missing account', () { + final now = DateTime(2026, 8, 10, 12); + final status = SyncHealthStatus( + lastSuccess: now.subtract(const Duration(days: 2)), + failureKind: SyncFailureKind.unconfigured, + now: now, + ); + expect(status.message, contains('no account connected')); + }); + + test('reports days for a long stall and hours for a short one', () { + final now = DateTime(2026, 8, 10, 12); + String? messageAfter(Duration since) => SyncHealthStatus( + lastSuccess: now.subtract(since), + failureKind: null, + now: now, + ).message; + + expect(messageAfter(const Duration(days: 3)), contains('3d ago')); + expect(messageAfter(const Duration(hours: 9)), contains('9h ago')); + }); + + test('a failure just after a success does not say "0h ago"', () { + // Regression guard: reporting elapsed time before any has elapsed + // rendered "Not syncing since 0h ago", which reads as a bug in the + // banner rather than a report about the sync. + final now = DateTime(2026, 8, 10, 12); + final status = SyncHealthStatus( + lastSuccess: now.subtract(const Duration(minutes: 5)), + failureKind: SyncFailureKind.failed, + now: now, + ); + expect(status.isStalled, isTrue); + expect(status.message, 'Not syncing. Meals stay on this device.'); + expect(status.message, isNot(contains('0h'))); + }); + + test('a failure with no prior success still reads sensibly', () { + final status = SyncHealthStatus( + lastSuccess: null, + failureKind: SyncFailureKind.failed, + now: DateTime(2026, 8, 10), + ); + expect(status.message, 'Not syncing. Meals stay on this device.'); + }); + }); + + group('SyncHealth persistence', () { + test('recordSuccess clears a previous failure', () async { + await SyncHealth.recordFailure(); + expect((await SyncHealth.read()).failureKind, SyncFailureKind.failed); + + await SyncHealth.recordSuccess(); + final status = await SyncHealth.read(); + expect(status.failureKind, isNull); + expect(status.lastSuccess, isNotNull); + expect(status.isStalled, isFalse); + }); + + test('recordUnconfigured round-trips', () async { + await SyncHealth.recordUnconfigured(); + expect( + (await SyncHealth.read()).failureKind, + SyncFailureKind.unconfigured, + ); + }); + + test('an unrecognised stored kind reads as no failure', () async { + SharedPreferences.setMockInitialValues({ + 'sync.health.failureKind': 'not-a-kind', + }); + expect((await SyncHealth.read()).failureKind, isNull); + }); + + test('resetForTest forgets everything', () async { + await SyncHealth.recordSuccess(); + await SyncHealth.resetForTest(); + final status = await SyncHealth.read(); + expect(status.lastSuccess, isNull); + expect(status.failureKind, isNull); + }); + }); +} diff --git a/app/test/widgets/sync_health_banner_test.dart b/app/test/widgets/sync_health_banner_test.dart new file mode 100644 index 0000000..567054a --- /dev/null +++ b/app/test/widgets/sync_health_banner_test.dart @@ -0,0 +1,71 @@ +import 'package:diet_guard_app/services/sync_health.dart'; +import 'package:diet_guard_app/widgets/sync_health_banner.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +Future pump(WidgetTester tester, SyncHealthStatus? status) => + tester.pumpWidget( + MaterialApp(home: Scaffold(body: SyncHealthBanner(status: status))), + ); + +void main() { + testWidgets('renders nothing before health has been read', (tester) async { + await pump(tester, null); + expect(find.byIcon(Icons.cloud_off), findsNothing); + }); + + testWidgets('renders nothing for a healthy device', (tester) async { + final now = DateTime(2026, 8, 10, 12); + await pump( + tester, + SyncHealthStatus( + lastSuccess: now.subtract(const Duration(minutes: 5)), + failureKind: null, + now: now, + ), + ); + expect(find.byIcon(Icons.cloud_off), findsNothing); + }); + + testWidgets('warns when the device has stopped publishing', (tester) async { + final now = DateTime(2026, 8, 10, 12); + await pump( + tester, + SyncHealthStatus( + lastSuccess: now.subtract(const Duration(days: 2)), + failureKind: SyncFailureKind.failed, + now: now, + ), + ); + + expect(find.byIcon(Icons.cloud_off), findsOneWidget); + expect( + find.textContaining('Meals stay on this device'), + findsOneWidget, + ); + }); + + testWidgets('survives a theme with no AppStatusColors extension', ( + tester, + ) async { + // Regression guard: a `!` on the extension lookup crashed every widget + // test that pumped a bare MaterialApp. + final now = DateTime(2026, 8, 10, 12); + await tester.pumpWidget( + MaterialApp( + theme: ThemeData(extensions: const []), + home: Scaffold( + body: SyncHealthBanner( + status: SyncHealthStatus( + lastSuccess: now.subtract(const Duration(days: 2)), + failureKind: SyncFailureKind.unconfigured, + now: now, + ), + ), + ), + ), + ); + expect(tester.takeException(), isNull); + expect(find.byIcon(Icons.cloud_off), findsOneWidget); + }); +} diff --git a/diet_guard/_cli_sync.py b/diet_guard/_cli_sync.py index 4b3adfc..5a7a4d3 100644 --- a/diet_guard/_cli_sync.py +++ b/diet_guard/_cli_sync.py @@ -10,7 +10,7 @@ from __future__ import annotations from typing import TYPE_CHECKING -from crdt_sync import GitHubSyncError +from crdt_sync import RemoteSyncError from diet_guard._sync import SyncError, run_sync @@ -48,7 +48,10 @@ def cmd_sync(emit: Callable[[str], None]) -> int: except SyncError as exc: emit(f"sync not configured: {exc}") return 1 - except GitHubSyncError as exc: + except RemoteSyncError as exc: + # The shared base, not GitHubSyncError: Firebase's errors are siblings + # of GitHub's, so catching only the latter let the primary backend's + # failures crash this timer-driven command with a traceback. emit(f"sync failed: {exc}") return 1 total_entries = sum(len(entries) for entries in merged.values()) diff --git a/diet_guard/_sync.py b/diet_guard/_sync.py index 1160215..6813310 100644 --- a/diet_guard/_sync.py +++ b/diet_guard/_sync.py @@ -31,6 +31,7 @@ from crdt_sync import ( RemoteStore, RemoteSyncError, SyncState, + firebase_client_for, merge_logs, mirror_client_for, revision_of, @@ -109,6 +110,47 @@ def _remote_client(github: GitHubSyncClient) -> RemoteStore: return github +def _client_for_run() -> RemoteStore: + """Return the backend for one sync run, requiring only *one* to be set up. + + The PAT is no longer mandatory. Firebase is the primary backend now, and + requiring a GitHub token before ever constructing it meant a + Firebase-configured machine with no PAT could not sync at all -- the + commits that fixed exactly this ("Auto-sync on a Firebase-only device") + changed only the Dart side, leaving the Python half behind. + + A missing PAT is therefore fatal only when Firebase is *also* unconfigured, + which is the genuine "nothing is set up" case the caller must report. + """ + try: + token = _read_token() + except SyncError: + if not CONFIG_FILE.is_file(): + raise + # Firebase-only: there is no GitHub client to mirror to, so use the + # primary directly rather than failing the whole run. + # + # ConfigError is translated rather than propagated: it is *not* a + # RemoteSyncError (it subclasses Exception directly), so letting it out + # would escape every caller's catch tuple and raise a traceback out of + # the gate's fail-closed "Fetch from sync" button -- the exact failure + # the RemoteSyncError swap was made to stop. A config file that exists + # but is unusable is reached by precisely this branch. + try: + return firebase_client_for("diet_guard") + except ConfigError as exc: + message = f"Firebase config at {CONFIG_FILE} is unusable: {exc}" + raise SyncError(message) from exc + return _remote_client( + GitHubSyncClient( + SYNC_REPO_OWNER, + SYNC_REPO_NAME, + token, + timeout_seconds=SYNC_TIMEOUT_SECONDS, + ) + ) + + def _device_log_path(device_id: str) -> str: """Return the repo-relative path a device's full log is pushed to.""" return f"{_DEVICES_DIR}/{device_id}/food_log.json" @@ -372,20 +414,13 @@ def run_sync() -> DayLog: The merged log as it now sits on disk locally, post re-sign. Raises: - SyncError: If the local PAT is missing or empty. - crdt_sync.GitHubSyncError: Propagated from the GitHub client for any - transport failure -- the caller (CLI/timer) decides how to + SyncError: If *neither* backend is configured -- no Firebase config and + no usable PAT. + crdt_sync.RemoteSyncError: Propagated from whichever backend failed for + any transport failure -- the caller (CLI/timer) decides how to report it. """ - token = _read_token() - client = _remote_client( - GitHubSyncClient( - SYNC_REPO_OWNER, - SYNC_REPO_NAME, - token, - timeout_seconds=SYNC_TIMEOUT_SECONDS, - ) - ) + client = _client_for_run() identity = device_identity() state_store = FileSyncStateStore(SYNC_STATE_FILE) @@ -443,14 +478,21 @@ def pull_shared_log() -> str | None: its own lock decision rather than failing open. The three caught types are the whole realistic failure surface of a run: - :class:`SyncError` (no/empty token), :class:`~crdt_sync.GitHubSyncError` - (the client wraps every ``requests`` transport error in this), and + :class:`SyncError` (no/empty token), :class:`~crdt_sync.RemoteSyncError` + (the shared base of every backend's transport/auth failure), and :class:`OSError` (reading the token or writing the merged log back). A bug outside these is deliberately *not* swallowed -- it should surface, not be silently reported as a sync outage. + + ``RemoteSyncError``, **not** ``GitHubSyncError``: since Firebase became the + primary backend, ``FirebaseSyncError``/``FirebaseAuthError`` are *siblings* + of ``GitHubSyncError`` under ``RemoteSyncError``, not subclasses. Catching + the GitHub type alone let every per-request Firebase failure escape, so + this fail-closed helper raised a traceback out of the gate's "Fetch from + sync" button instead of returning a reason and leaving the lock up. """ try: run_sync() - except (SyncError, GitHubSyncError, OSError) as exc: + except (SyncError, RemoteSyncError, OSError) as exc: return f"sync unavailable ({exc})" return None diff --git a/diet_guard/tests/_gate_keyboard_probe.py b/diet_guard/tests/_gate_keyboard_probe.py index 9735aa4..d6e7aa3 100644 --- a/diet_guard/tests/_gate_keyboard_probe.py +++ b/diet_guard/tests/_gate_keyboard_probe.py @@ -76,6 +76,13 @@ def probe(screen_w: int = 1366, screen_h: int = 768) -> RingReport: """Build the gate and probe its keyboard behaviour.""" root = tk.Tk() try: + # Bypass the window manager -- see the same call in + # `_gate_layout_probe.measure`. A tiling WM retiles this toplevel + # instead of honouring the geometry, the widgets never realize at a + # usable size, and the focus ring then walks a single stop, failing + # every traversal assertion for reasons that have nothing to do with + # the focus order under test. + root.overrideredirect(boolean=True) root.geometry(f"{screen_w}x{screen_h}+0+0") notebook = ttk.Notebook(root) notebook.place(relx=0, rely=0, relwidth=1, relheight=1) diff --git a/diet_guard/tests/_gate_layout_probe.py b/diet_guard/tests/_gate_layout_probe.py index 971632d..22e0bbe 100644 --- a/diet_guard/tests/_gate_layout_probe.py +++ b/diet_guard/tests/_gate_layout_probe.py @@ -281,6 +281,16 @@ def measure( """ root = tk.Tk() try: + # Bypass the window manager. This measures whether the layout fits a + # given panel, so the window must actually BE that size -- but a + # tiling WM (i3 on the dev machine) retiles a managed toplevel to the + # workspace instead, and `winfo_height()` then reports 0/1px. Every + # measurement taken from that is noise, and the assertions fail on it + # rather than on any real layout regression. `overrideredirect` takes + # the window out of the WM's control, so the requested geometry is + # honoured exactly -- verified 1366x768 under i3, and unchanged under + # a bare Xvfb, which has no WM to bypass in the first place. + root.overrideredirect(boolean=True) root.geometry(f"{screen_w}x{screen_h}+0+0") _notebook_overhead(root) notebook = ttk.Notebook(root) diff --git a/diet_guard/tests/test_sync.py b/diet_guard/tests/test_sync.py index 2c26da2..8d5a803 100644 --- a/diet_guard/tests/test_sync.py +++ b/diet_guard/tests/test_sync.py @@ -8,10 +8,14 @@ The GitHub layer is mocked (no network access); conftest.py's from __future__ import annotations import json -from pathlib import Path from unittest.mock import MagicMock, patch -from crdt_sync import ConfigError, GitHubSyncError, SyncState +from crdt_sync import ( + FirebaseAuthError, + FirebaseSyncError, + GitHubSyncError, + SyncState, +) import pytest from diet_guard import _sync @@ -364,6 +368,30 @@ class TestPullSharedLog: assert reason is not None assert "boom" in reason + @pytest.mark.parametrize( + "error", + [ + FirebaseAuthError("token refresh rejected"), + FirebaseSyncError("RTDB unavailable"), + ], + ) + def test_firebase_failure_fails_closed(self, error: Exception) -> None: + """A Firebase failure must return a reason, never raise. + + Regression guard: ``FirebaseAuthError``/``FirebaseSyncError`` are + *siblings* of ``GitHubSyncError`` under ``RemoteSyncError``, not + subclasses. Catching only the GitHub type let every per-request + failure of the *primary* backend escape, so the gate's "Fetch from + sync" button raised a traceback instead of reporting a reason and + leaving the lock up -- on the exact path this helper exists to + fail closed. + """ + with patch.object(_sync, "run_sync", side_effect=error): + reason = _sync.pull_shared_log() + + assert reason is not None + assert str(error) in reason + def test_unexpected_error_is_not_swallowed(self) -> None: """A bug outside the known failure surface surfaces, not hidden.""" with ( @@ -373,50 +401,6 @@ class TestPullSharedLog: _sync.pull_shared_log() -class TestRemoteClient: - """Which backend a sync tick runs against during the cutover.""" - - def test_stays_on_github_without_firebase_config( - self, monkeypatch: pytest.MonkeyPatch - ) -> None: - """An unconfigured machine must not reach the network at all.""" - monkeypatch.setattr(_sync, "CONFIG_FILE", Path("/nonexistent/firebase.json")) - github = object() - - assert _sync._remote_client(github) is github - - def test_mirrors_to_github_when_configured( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch - ) -> None: - """Configured: Firebase is primary, GitHub keeps receiving writes.""" - config = tmp_path / "firebase.json" - config.write_text("{}", encoding="utf-8") - monkeypatch.setattr(_sync, "CONFIG_FILE", config) - monkeypatch.setattr( - _sync, "mirror_client_for", lambda _app, client: ("mirror", client) - ) - github = object() - - assert _sync._remote_client(github) == ("mirror", github) - - def test_falls_back_when_firebase_is_unusable( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch - ) -> None: - """A broken Firebase must degrade to GitHub, never fail the tick.""" - config = tmp_path / "firebase.json" - config.write_text("{}", encoding="utf-8") - monkeypatch.setattr(_sync, "CONFIG_FILE", config) - - def _boom(*_args: object, **_kwargs: object) -> None: - message = "no password" - raise ConfigError(message) - - monkeypatch.setattr(_sync, "mirror_client_for", _boom) - github = object() - - assert _sync._remote_client(github) is github - - class TestRemoteRevisions: """Reading the peers' revision map, which gates the big downloads.""" diff --git a/diet_guard/tests/test_sync_backend.py b/diet_guard/tests/test_sync_backend.py new file mode 100644 index 0000000..7c7a6c9 --- /dev/null +++ b/diet_guard/tests/test_sync_backend.py @@ -0,0 +1,117 @@ +"""Tests for which backend a sync tick runs against. + +Split out of ``test_sync.py`` to keep both files under the repo's 500-line +cap. Covers the Firebase/GitHub cutover seam (``_remote_client``) and which +credentials a run actually requires (``_client_for_run``). +""" + +from __future__ import annotations + +from pathlib import Path + +from crdt_sync import ConfigError +import pytest + +from diet_guard import _sync + + +class TestRemoteClient: + """Which backend a sync tick runs against during the cutover.""" + + def test_stays_on_github_without_firebase_config( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """An unconfigured machine must not reach the network at all.""" + monkeypatch.setattr(_sync, "CONFIG_FILE", Path("/nonexistent/firebase.json")) + github = object() + + assert _sync._remote_client(github) is github + + def test_mirrors_to_github_when_configured( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Configured: Firebase is primary, GitHub keeps receiving writes.""" + config = tmp_path / "firebase.json" + config.write_text("{}", encoding="utf-8") + monkeypatch.setattr(_sync, "CONFIG_FILE", config) + monkeypatch.setattr( + _sync, "mirror_client_for", lambda _app, client: ("mirror", client) + ) + github = object() + + assert _sync._remote_client(github) == ("mirror", github) + + def test_falls_back_when_firebase_is_unusable( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A broken Firebase must degrade to GitHub, never fail the tick.""" + config = tmp_path / "firebase.json" + config.write_text("{}", encoding="utf-8") + monkeypatch.setattr(_sync, "CONFIG_FILE", config) + + def _boom(*_args: object, **_kwargs: object) -> None: + message = "no password" + raise ConfigError(message) + + monkeypatch.setattr(_sync, "mirror_client_for", _boom) + github = object() + + assert _sync._remote_client(github) is github + + +class TestClientForRun: + """Which credentials a run actually requires.""" + + def test_syncs_without_a_pat_when_firebase_is_configured( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A Firebase-only machine must sync, not fail for a missing PAT. + + Requiring the token before ever constructing Firebase left the Python + half unable to sync at all on a device that had moved to Firebase -- + the "Firebase-only device" fix had only ever touched the Dart side. + """ + config = tmp_path / "firebase.json" + config.write_text("{}", encoding="utf-8") + monkeypatch.setattr(_sync, "CONFIG_FILE", config) + monkeypatch.setattr(_sync, "SYNC_TOKEN_FILE", tmp_path / "absent_sync_token") + monkeypatch.setattr(_sync, "firebase_client_for", lambda app: ("firebase", app)) + + assert _sync._client_for_run() == ("firebase", "diet_guard") + + def test_unusable_firebase_config_becomes_a_sync_error( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """An unusable Firebase config must fail *closed*, not raise. + + ``ConfigError`` subclasses ``Exception`` directly, not + ``RemoteSyncError``, so letting it out of here escapes every caller's + catch tuple and raises a traceback out of the gate's fail-closed + "Fetch from sync" button -- reintroducing, by a different exception, + exactly the failure the ``RemoteSyncError`` swap was made to stop. + This branch is reached precisely when the config file exists but is + unusable and no PAT is present. + """ + config = tmp_path / "firebase.json" + config.write_text("{}", encoding="utf-8") + monkeypatch.setattr(_sync, "CONFIG_FILE", config) + monkeypatch.setattr(_sync, "SYNC_TOKEN_FILE", tmp_path / "absent_token") + + def _boom(*_args: object, **_kwargs: object) -> None: + message = "no password" + raise ConfigError(message) + + monkeypatch.setattr(_sync, "firebase_client_for", _boom) + + with pytest.raises(_sync.SyncError): + _sync._client_for_run() + + def test_still_fails_when_neither_backend_is_configured( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """No PAT *and* no Firebase is the genuine "nothing is set up" case.""" + monkeypatch.setattr(_sync, "CONFIG_FILE", tmp_path / "absent_firebase.json") + monkeypatch.setattr(_sync, "SYNC_TOKEN_FILE", tmp_path / "absent_sync_token") + + with pytest.raises(_sync.SyncError): + _sync._client_for_run()