diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 34f39a2..5f9c694 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -153,6 +153,14 @@ repos: entry: python3 scripts/check_silent_failures.py language: system types: [python] + - id: no-silent-failures-dart + # The Python hook above is types:[python] and so had never looked at a + # single Dart file — in the app where a swallowed sync error costs the + # user a workout with nothing anywhere saying so. + name: Block silent failures in Dart (swallowed errors must log why) + entry: python3 scripts/check_silent_failures_dart.py + language: system + types: [dart] - id: screens-fit name: Every lock screen fits 1366x768 (no scrolling required) entry: python3 -m scripts.verify_screen_fits diff --git a/docs/android-ui-automation-task.md b/docs/android-ui-automation-task.md new file mode 100644 index 0000000..db44537 --- /dev/null +++ b/docs/android-ui-automation-task.md @@ -0,0 +1,124 @@ +# Task: replace blind coordinate tapping with element-targeted Android UI automation + +> **STATUS 2026-08-10: BUILT.** The tool now exists at +> `~/testsAndMisc/python_pkg/android_ui/` (see its README). It covers `dump` / +> `find` / `tap` / `type` / `wait` / `focus` with verified typing, ambiguity as +> an error, keyboard detection, and partial-tree retries — all exercised +> against the physical Pixel 6a. What remains from the spec below is the +> optional MCP wrapper, `scroll_to()`, and rolling it out to `~/todo` and +> `~/dufs-cloud/app`. Everything after this line is the original brief, kept +> because it records WHY each guarantee exists. + +## what + +Every Flutter/Android app in these repos is currently driven by an agent the +same bad way: screenshot -> read pixel positions by eye -> `adb shell input tap + `. Replace that with a reusable tool that finds elements by *identity* +(text, content-desc, resource-id) and taps their real bounds, and that can +report what is on screen as structured text instead of an image. + +## why this is not a nicety + +Observed in one session (2026-08-10, workout_app Firebase verification): + +- **Silent mis-taps.** A tap at `(540, 1705)` intended for the password field + landed with the field unfocused. The typed password went nowhere. Nothing + errored — the next screenshot just showed an empty box. A no-op and a hit are + indistinguishable without dumping the tree and diffing. +- **Layout shift invalidates every coordinate.** Opening the soft keyboard moved + the email field from y=1572 to y=1319. Coordinates captured one step earlier + were already wrong. +- **Screenshot scaling is a manual multiply.** The harness renders 1080x2400 at + 900x2000, so every read coordinate needs `* 1.20`. One forgotten multiply is + an off-screen tap. +- **Screenshots are expensive.** A full-page PNG per step burns far more context + than the equivalent element list, and can't be grepped or asserted on. + +`uiautomator dump` already solves this and is on every device — no new +dependency: + +```bash +adb shell uiautomator dump /sdcard/ui.xml +adb pull /sdcard/ui.xml +# -> +``` + +This is how the same session recovered: dumping the tree found both `EditText` +nodes by class and gave exact centers, and the retry worked first time. + +## where + +New shared tool, not a per-repo copy. Two options, pick one: + +- `~/testsAndMisc/python_pkg/android_ui/` — a CLI (`android-ui find "Connect + Firebase"`, `android-ui tap "Connect Firebase"`, `android-ui dump`), matching + how `app_icons` is already shared across repos. +- An MCP server exposing the same as tools, which is what makes it available to + the agent without shelling out. Preferred if MCP is acceptable — the user + asked for "some tool ... (mcp?) which also translates to UI". + +Consumers (every repo with an Android surface): +- `~/screen-locker/stronglift_replacement/workout_app` +- `~/todo` +- `~/dufs-cloud/app` +- any future Flutter app (this should be part of the app scaffold) + +## must + +- `find(query)` -> matching elements with class, text/content-desc, bounds, + center, and enabled/focused state. Match on text, content-desc, or + resource-id; substring and exact. +- `tap(query)` -> resolves the query, taps the center of the *current* bounds + (re-dump before every tap; never reuse a stale coordinate), and **fails + loudly with a non-zero exit when the query matches zero or >1 elements**. + Ambiguity must be an error, not a silent first-match. +- `type(query, text)` -> focus the field, type, then **verify the field's text + actually changed** and fail if not. This is the specific bug that cost this + session a round trip. +- `dump()` -> flat, greppable text list of visible elements. This replaces the + screenshot for most steps. +- Waits: `wait_for(query, timeout)` that polls the tree, so no more blind + `sleep 14`. +- must not: require root, an emulator, a running Flutter debug connection, or + any change to app source. It must work against a plain release APK on a + physical device over adb. +- optional: `flutter drive` / integration_test as a second backend when a debug + build IS available — richer (widget keys, no dump latency), but it cannot + drive a release APK, so it can't be the only path. +- optional: screenshot fallback for genuinely visual checks (theming, icon + rendering) — where a picture is the point, not a workaround. + +## done + +A verification script that drives workout_app's "connect Firebase" flow +end-to-end with **zero hardcoded pixel coordinates** in it, and: + +- passes on the current layout; +- still passes after the SYNC and OFFLINE BACKUP sections are reordered in + `settings_screen.dart` (the test that coordinate-based automation fails); +- exits non-zero, naming the query, when an element is missing. + +## verify + +On the physical Pixel 6a over adb, against an installed release APK — not an +emulator and not a debug-only Flutter driver session. + +## read first + +- This session's working recovery: `uiautomator dump` + the bounds-parsing + python in the screen-locker scratchpad (`ui.xml`, `ui2.xml`). +- `~/screen-locker/CLAUDE.md` — "You are NOT done until you install the new + version on the phone itself", which is why agent-driven UI is on the critical + path for every task in this repo. +- Note `uiautomator dump` returns an empty/partial tree while an animation is + in flight; retry-with-backoff belongs in the tool, not in each caller. +- **Observed 2026-08-10, must be handled:** while a Flutter `TextField` holds + focus, the dump returns ONLY the `EditText` nodes — the rest of the tree + (including the button you are about to tap) is missing, and it stays missing + across retries, so a naive retry loop never converges. Worse, `KEYCODE_BACK` + to dismiss the keyboard is treated by Flutter as a route pop: it navigated + out of Settings and discarded the typed credentials, twice. The tool must + unfocus without popping the route (tap dead space, or `dumpsys input_method` + to check `mInputShown` first) and must distinguish "tree is partial" from + "element is genuinely absent" rather than reporting a false negative. diff --git a/scripts/check_silent_failures_dart.py b/scripts/check_silent_failures_dart.py new file mode 100755 index 0000000..8d2ad3b --- /dev/null +++ b/scripts/check_silent_failures_dart.py @@ -0,0 +1,207 @@ +#!/usr/bin/env python3 +"""Pre-commit hook: ban silent failures in Dart, mirroring the Python hook. + +``check_silent_failures.py`` has enforced this rule for Python since the PC's +workout sync silently did nothing for weeks. It is gated on ``types: [python]``, +so it has never looked at a single Dart file — and the workout app is where the +same class of bug is most expensive, because a swallowed sync error there costs +the user a workout with nothing anywhere saying so. + +The rule matches the Python hook: every ``catch`` must either rethrow or say +what happened somewhere a human will actually see. In Dart that means + +* ``rethrow`` / ``throw`` (the failure is someone else's problem), or +* ``log(...)`` from ``dart:developer`` -- the release-safe channel, and the one + the sync services already use at ``level: 1000``, or +* ``debugPrint(...)`` -- accepted because this codebase uses it widely for + operator-facing storage/backup diagnostics, even though it is stripped in + release builds. + +``print()`` does NOT count: it is lint-banned in this repo and vanishes in +release. A bare ``catchError((_) {})`` is the Dart spelling of ``except: pass`` +and is always reported. + +Deliberately regex/brace based rather than a real Dart parser: the alternative +is making `analyzer` a hook dependency, and the shapes this needs to recognise +are narrow. It errs toward reporting -- an unparsable handler is a finding, +not a skip, exactly like the Python hook's SyntaxError branch. +""" + +from __future__ import annotations + +import logging +from pathlib import Path +import re +import sys + +_logger = logging.getLogger(__name__) + +# `log` is dart:developer's; `debugPrint` is flutter/foundation's. +# +# `_setSyncStatus` / `ScaffoldMessenger` count as loud on purpose: telling the +# USER "could not start device flow: " in the UI is the loudest channel +# there is. The rule is "the failure must be visible", not "it must reach a +# log file", and a screen that reports the error is not a silent failure. +_LOUD_CALLS = ( + "log(", + "debugPrint(", + "rethrow", + "throw ", + "_setSyncStatus(", + "ScaffoldMessenger", +) + +# `} on Foo catch (e) {`, `} catch (e) {`, `on Foo {`. +_CATCH_RE = re.compile(r"\bcatch\s*\(|\bon\s+\w+\s*\{") + +# `.catchError((_) {})` and friends -- an empty body is always silent. +_EMPTY_CATCH_ERROR_RE = re.compile(r"\.catchError\(\s*\([^)]*\)\s*(?:=>\s*)?\{\s*\}") + + +def _strip_noise(source: str) -> str: + """Blank out comments and string literals so they cannot match as code. + + A comment mentioning ``log(`` would otherwise make a silent handler look + loud -- the exact false negative that lets the bug back in. + """ + out: list[str] = [] + i = 0 + n = len(source) + while i < n: + two = source[i : i + 2] + if two == "//": + end = source.find("\n", i) + end = n if end == -1 else end + out.append(" " * (end - i)) + i = end + elif two == "/*": + end = source.find("*/", i + 2) + end = n if end == -1 else end + 2 + # Keep newlines so line numbers stay correct. + out.append("".join(c if c == "\n" else " " for c in source[i:end])) + i = end + elif source[i] in "\"'": + quote = source[i] + j = i + 1 + while j < n and source[j] != quote: + if source[j] == "\\": + j += 1 + if source[j : j + 1] == "\n": + break + j += 1 + j = min(j + 1, n) + out.append("".join(c if c == "\n" else " " for c in source[i:j])) + i = j + else: + out.append(source[i]) + i += 1 + return "".join(out) + + +def _block_after(source: str, start: int) -> tuple[str, int] | None: + """Return the ``{...}`` block that follows ``start``, and its end offset.""" + brace = source.find("{", start) + if brace == -1: + return None + depth = 0 + for idx in range(brace, len(source)): + if source[idx] == "{": + depth += 1 + elif source[idx] == "}": + depth -= 1 + if depth == 0: + return source[brace : idx + 1], idx + return None + + +def _handler_hands_error_on(body: str, catch_clause: str) -> bool: + """Return True if the handler passes the caught error to its caller. + + ``} on GitHubSyncError catch (e) { return e; }`` is not a silent failure: + the error becomes the function's result, and whoever asked for it decides + how loud to be. Same for stashing it in state the UI renders + (``setState(() => _error = '$e')``) or reassigning a variable that is + logged after the loop. Reporting these would push the codebase toward + duplicate logging, which is its own kind of noise. + """ + name = re.search(r"catch\s*\(\s*(\w+)", catch_clause) + if name is None: + return False + var = name.group(1) + return bool( + # `return e;` + re.search(rf"\breturn\s+{var}\b", body) + # `failure = retryError;` — reassigned for the caller to report. + or re.search(rf"=\s*{var}\s*[;,)]", body) + # `setState(() => _error = '$e')` — rendered by the widget. + or re.search(rf"=\s*'[^']*\$\{{?{var}\b", body) + ) + + +def _check_source(path: Path, source: str) -> list[str]: + """Return one message per silently-swallowing catch in ``source``.""" + code = _strip_noise(source) + problems: list[str] = [] + + for match in _EMPTY_CATCH_ERROR_RE.finditer(code): + line = code.count("\n", 0, match.start()) + 1 + problems.append( + f"{path}:{line}: .catchError with an empty body swallows the error " + f"without logging why — log(...) the reason, or rethrow" + ) + + for match in _CATCH_RE.finditer(code): + block = _block_after(code, match.start()) + if block is None: + line = code.count("\n", 0, match.start()) + 1 + problems.append( + f"{path}:{line}: could not find the catch body — reporting it " + f"rather than skipping, since it cannot be checked" + ) + continue + body, _end = block + # The clause itself (`catch (retryError)`) names the variable; slice it + # from the match rather than searching the file, which would find the + # FIRST catch in the file for every handler. + clause = code[match.start() : code.find("{", match.start()) + 1] + # Check the RAW body too: `_error = '$e'` hands the error to the UI, + # but _strip_noise blanks the string literal it lives in, so the + # stripped body alone cannot see it. + raw_body = source[match.start() : match.start() + len(body) + 200] + if _handler_hands_error_on(body, clause) or _handler_hands_error_on( + raw_body, clause + ): + continue + if not any(token in body for token in _LOUD_CALLS): + line = code.count("\n", 0, match.start()) + 1 + problems.append( + f"{path}:{line}: catch swallows the error without logging why " + f"— rethrow, or log(..., level: 1000) / debugPrint the reason" + ) + return problems + + +def main() -> int: + """Return 1 if any Dart file swallows an error silently, else 0.""" + problems: list[str] = [] + for filename in sys.argv[1:]: + path = Path(filename) + try: + source = path.read_text(encoding="utf-8", errors="replace") + except OSError as exc: + _logger.warning( + "Could not read %s (%s) — reporting it as a problem rather " + "than skipping it, since its handlers cannot be checked", + path, + exc, + ) + problems.append(f"{path}: could not read ({exc})") + continue + problems.extend(_check_source(path, source)) + for problem in problems: + sys.stdout.write(f"{problem}\n") + return 1 if problems else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/stronglift_replacement/workout_app/lib/main.dart b/stronglift_replacement/workout_app/lib/main.dart index 59f38e1..1418ea3 100644 --- a/stronglift_replacement/workout_app/lib/main.dart +++ b/stronglift_replacement/workout_app/lib/main.dart @@ -4,6 +4,7 @@ import 'package:flutter/material.dart'; import 'package:workout_app/screens/home_screen.dart'; import 'package:workout_app/services/backup_service.dart'; import 'package:workout_app/services/http_server_service.dart'; +import 'package:workout_app/services/progression_sync_service.dart'; import 'package:workout_app/services/storage_service.dart'; import 'package:workout_app/services/sync_device_id.dart'; import 'package:workout_app/ui/theme.dart'; @@ -17,22 +18,50 @@ void main() async { // Before anything that stamps an Hlc or syncs: until this resolves, the // device id falls back to the pre-migration constant. await initSyncDeviceId(); - // Storage permission MUST be settled before the DB is opened: opening it - // seeds factory defaults for any missing exercise, and restoring afterwards - // is a no-op once rows exist. Losing the race means a reinstall silently - // comes up at defaults with months of progression gone -- which is what - // happened on 2026-08-05. - final storageGranted = await BackupService.instance - .requestStoragePermission(); + // Storage permission is settled before the DB is opened: opening it seeds + // factory defaults for any missing exercise, and restoring afterwards is a + // no-op once rows exist. Losing that race is what wiped months of + // progression on 2026-08-05. + // + // CHECKED, never requested. `request()` throws the user into a system + // Settings page on every launch where the permission is absent, and since + // Firebase now carries progression the `/sdcard` backup is a second copy + // rather than the only one. The grant prompt lives in Settings, where the + // user asks for it. + final storageGranted = await BackupService.instance.hasStoragePermission(); if (!storageGranted) { debugPrint( - 'WorkoutApp: storage permission DENIED — the backup at $kBackupPath can ' - 'neither be read nor written. Progression cannot be restored on this ' - 'launch and will not be protected against the next reinstall.', + 'WorkoutApp: no storage permission — the backup at $kBackupPath can ' + 'neither be read nor written, so progression rides on Firebase alone. ' + 'Grant it in Settings to keep a second, offline copy.', ); } await StorageService.init(); await StorageService.instance.restoreFromBackupIfNeeded(); + // The permission-free restore path, and the reason storage is now optional. + // `backup.json` is unreadable without the grant, so on a denied reinstall the + // line above is a no-op and this is the ONLY thing between the user and + // factory defaults. Safe to run unconditionally: it applies remote records + // only to a freshly-installed DB, so it restores a wiped install but never + // overwrites progression — or a deliberate reset — made on this phone. + // + // Bounded: this is N sequential network reads on the launch path, so a slow + // or half-open connection would otherwise hold the UI on a blank screen. + // Timing out is safe — it leaves `progression_synced_at` unset, so the next + // launch retries and, until it succeeds, pushProgression refuses to + // overwrite the remote copy. + final restored = await ProgressionSyncService() + .pullProgression() + .timeout( + const Duration(seconds: 20), + onTimeout: () => const ProgressionSyncResult( + changed: false, + reason: + 'progression pull timed out after 20s — starting on local state; ' + 'the next launch will retry', + ), + ); + debugPrint('WorkoutApp: ${restored.reason}'); await HttpServerService.instance.start(); runApp(const WorkoutApp()); } diff --git a/stronglift_replacement/workout_app/lib/screens/settings_screen.dart b/stronglift_replacement/workout_app/lib/screens/settings_screen.dart index cedef24..31c17a4 100644 --- a/stronglift_replacement/workout_app/lib/screens/settings_screen.dart +++ b/stronglift_replacement/workout_app/lib/screens/settings_screen.dart @@ -10,8 +10,10 @@ import 'package:http/http.dart' as http; import 'package:url_launcher/url_launcher.dart'; import 'package:workout_app/models/exercise.dart'; import 'package:workout_app/models/workout_plan.dart'; +import 'package:workout_app/services/backup_service.dart'; import 'package:workout_app/services/firebase_backend.dart'; import 'package:workout_app/services/github_device_auth.dart'; +import 'package:workout_app/services/progression_sync_service.dart'; import 'package:workout_app/services/storage_service.dart'; import 'package:workout_app/services/sync_settings.dart'; import 'package:workout_app/ui/theme.dart'; @@ -29,6 +31,9 @@ class SettingsScreen extends StatefulWidget { this.accountLoader, this.accountSaver, this.accountClearer, + this.storageChecker, + this.storageRequester, + this.progressionPuller, }); /// Injectable HTTP client; tests pass a `MockClient` so the device-flow @@ -49,6 +54,17 @@ class SettingsScreen extends StatefulWidget { /// Forgets the account and any cached session. See [accountLoader]. final Future Function()? accountClearer; + /// Reads whether storage permission is held. Injected for the same reason as + /// [accountLoader]: `permission_handler` is a platform channel, and calling + /// it unguarded in `initState` made every settings-screen test throw. + final Future Function()? storageChecker; + + /// Opens the system grant page. See [storageChecker]. + final Future Function()? storageRequester; + + /// Pulls progression after a successful connect. See [storageChecker]. + final Future Function()? progressionPuller; + @override State createState() => _SettingsScreenState(); } @@ -71,6 +87,7 @@ class _SettingsScreenState extends State { final _emailController = TextEditingController(); final _passwordController = TextEditingController(); bool _firebaseConnected = false; + bool _storageGranted = false; bool _firebaseBusy = false; // Persistent (not a transient SnackBar) so the result of Connect GitHub / @@ -91,6 +108,28 @@ class _SettingsScreenState extends State { super.initState(); unawaited(_load()); unawaited(_loadFirebaseAccount()); + unawaited(_loadStorageGranted()); + } + + Future _loadStorageGranted() async { + final granted = + await (widget.storageChecker ?? + BackupService.instance.hasStoragePermission)(); + if (!mounted) return; + setState(() => _storageGranted = granted); + } + + /// Opens the system page for MANAGE_EXTERNAL_STORAGE. + /// + /// The ONLY caller of [BackupService.requestStoragePermission]: startup used + /// to call it and dumped the user in system Settings on every launch, for a + /// permission that is now genuinely optional. + Future _grantStorage() async { + final granted = + await (widget.storageRequester ?? + BackupService.instance.requestStoragePermission)(); + if (!mounted) return; + setState(() => _storageGranted = granted); } /// Reflects a previously-stored account, so a returning user sees the real @@ -137,11 +176,28 @@ class _SettingsScreenState extends State { return; } _passwordController.clear(); + + // Pull immediately. Startup already ran its pull and skipped, because at + // that point there was no account — so without this the device stays on + // factory defaults holding real remote progression, and its first finished + // workout would push those defaults over the top. Connecting late is the + // normal path after a reinstall (the uninstall wipes the keystore), so + // this is the common case, not an edge one. + final restored = await (widget.progressionPuller ?? + ProgressionSyncService().pullProgression)(); + if (!mounted) return; + setState(() { _firebaseBusy = false; _firebaseConnected = true; }); - _setSyncStatus('Connected to Firebase.', _SyncStatusKind.success); + _setSyncStatus( + restored.changed + ? 'Connected. Restored ${restored.count} exercise(s) from Firebase.' + : 'Connected to Firebase.', + _SyncStatusKind.success, + ); + if (restored.changed) await _load(); } Future _disconnectFirebase() async { @@ -573,6 +629,41 @@ class _SettingsScreenState extends State { ), ], ), + const SizedBox(height: 20), + const _SectionHeader('OFFLINE BACKUP'), + const SizedBox(height: 4), + Text( + _storageGranted + ? 'Granted. Progression is also written to ' + '$kBackupPath, so it survives a reinstall even ' + 'with no network.' + : 'Optional. Progression is restored from Firebase on ' + 'a fresh install, so this is a second, offline ' + 'copy — not a requirement. Granting it also keeps ' + 'a readable snapshot at $kBackupPath.', + style: TextStyle( + color: colorScheme.onSurfaceVariant, + fontSize: AppTextSize.caption, + ), + ), + const SizedBox(height: 12), + if (_storageGranted) + const Row( + children: [ + Icon(Icons.check_circle, size: 20), + SizedBox(width: 8), + Expanded(child: Text('Storage permission granted')), + ], + ) + else + Align( + alignment: Alignment.centerLeft, + child: ElevatedButton.icon( + onPressed: _grantStorage, + icon: const Icon(Icons.sd_storage), + label: const Text('Grant storage permission'), + ), + ), ], ), ); diff --git a/stronglift_replacement/workout_app/lib/screens/workout_screen.dart b/stronglift_replacement/workout_app/lib/screens/workout_screen.dart index d484313..9c8a5b2 100644 --- a/stronglift_replacement/workout_app/lib/screens/workout_screen.dart +++ b/stronglift_replacement/workout_app/lib/screens/workout_screen.dart @@ -11,6 +11,7 @@ import 'package:workout_app/models/exercise.dart'; import 'package:workout_app/models/exercise_result.dart'; import 'package:workout_app/models/set_result.dart'; import 'package:workout_app/models/workout_session.dart'; +import 'package:workout_app/services/progression_sync_service.dart'; import 'package:workout_app/services/storage_service.dart'; import 'package:workout_app/services/sync_service.dart'; import 'package:workout_app/services/workout_sync_service.dart'; @@ -145,8 +146,35 @@ class _WorkoutScreenState extends State { // ── Persistence ──────────────────────────────────────────────────────────── - Future _saveActiveSession() async { - await StorageService.instance.saveActiveSession({ + /// The most recent remote active-session write, so the clear-on-finish can + /// be ordered after it instead of racing it. + Future _lastActiveSessionPush = Future.value(); + + /// Persists the active session locally, and to Firebase when [toFirebase]. + /// + /// [toFirebase] is false on the per-tap paths (rep decrements, break + /// bookkeeping) and true only on set/warmup completion. `saveActiveSession` + /// runs on every tap, and a Firebase write per tap is exactly the traffic the + /// sync revision cache exists to avoid — so the remote copy is debounced to + /// the events that actually change which set the user is standing on. + Future _saveActiveSession({bool toFirebase = false}) async { + final data = _activeSessionData(); + await StorageService.instance.saveActiveSession(data); + if (!toFirebase) return; + _lastActiveSessionPush = ProgressionSyncService() + .pushActiveSession(data) + .then((result) { + if (!result.changed) { + debugPrint( + 'WorkoutScreen: active session not shared — ${result.reason}', + ); + } + }); + await _lastActiveSessionPush; + } + + Map _activeSessionData() { + return { 'workoutType': widget.workoutType, 'startTimeMs': _startTime.millisecondsSinceEpoch, 'tapped': _tapped, @@ -161,7 +189,7 @@ class _WorkoutScreenState extends State { .add(Duration(seconds: _breakDurationSecs)) .millisecondsSinceEpoch : 0, - }); + }; } // ── Helpers ──────────────────────────────────────────────────────────────── @@ -216,7 +244,9 @@ class _WorkoutScreenState extends State { } } - unawaited(_saveActiveSession()); + // Only a newly-completed set moves the workout forward; a rep decrement + // re-enters here and must not cost a remote write. + unawaited(_saveActiveSession(toFirebase: wasNotTapped)); } void _tapWarmup(int exIdx) { @@ -225,7 +255,7 @@ class _WorkoutScreenState extends State { if (!_inBreak) { _startBreak(_warmupBreakSecs, 'Warmup rest (3 min)', exIdx, -1); } - unawaited(_saveActiveSession()); + unawaited(_saveActiveSession(toFirebase: true)); } void _resetCircle(int exIdx, int setIdx) { @@ -300,7 +330,14 @@ class _WorkoutScreenState extends State { } Future _onBreakFinished() async { - await _audio.play(AssetSource('sounds/break_end.mp3')).catchError((_) {}); + await _audio + .play(AssetSource('sounds/break_end.mp3')) + .catchError((Object error) { + // Never fatal: a missing audio route must not interrupt the workout. + // But it is the break-end cue, so a silent failure looks like the + // timer itself is broken. + debugPrint('WorkoutApp: break-end sound failed to play ($error).'); + }); if (await Vibration.hasVibrator()) { // Android/iOS-only: hasVibrator() returns false on the Linux test host // (no Platform.isAndroid/isIOS), so this body never runs there. @@ -464,6 +501,29 @@ class _WorkoutScreenState extends State { await storage.setLastWorkoutType(widget.workoutType); await storage.clearActiveSession(); + // applyProgression just moved weights/reps/streaks, so this is the moment + // the remote copy goes stale. Pushed here (not on every set) because a + // finished workout is the only thing that changes progression. + unawaited( + ProgressionSyncService().pushProgression().then((result) { + if (!result.changed) { + log('Progression not synced: ${result.reason}', level: 1000); + } + }), + ); + // The workout is over: retract the shared in-progress session so another + // device cannot resume a session that no longer exists. + // + // Chained onto the in-flight publish rather than fired alongside it: the + // last set's `_saveActiveSession(toFirebase: true)` is unawaited, so two + // concurrent PUTs to the same path can land out of order and strand a + // finished session that the next install would faithfully restore. + unawaited( + _lastActiveSessionPush.then( + (_) => ProgressionSyncService().pushActiveSession(null), + ), + ); + final syncResult = await _sync.writeWorkoutResult(session); // Not awaited: a slow or unreachable backend must not delay the summary // dialog. But the result is no longer discarded -- a failed push logs at diff --git a/stronglift_replacement/workout_app/lib/services/backup_service.dart b/stronglift_replacement/workout_app/lib/services/backup_service.dart index 37ea3ee..f903f4d 100644 --- a/stronglift_replacement/workout_app/lib/services/backup_service.dart +++ b/stronglift_replacement/workout_app/lib/services/backup_service.dart @@ -15,6 +15,7 @@ library; import 'dart:convert'; import 'dart:io'; import 'package:flutter/foundation.dart' show debugPrint, visibleForTesting; +import 'package:flutter/services.dart' show MissingPluginException; import 'package:permission_handler/permission_handler.dart'; /// Default directory backups live in on external storage. @@ -69,13 +70,37 @@ class BackupService { // Thin wrappers over the permission_handler platform channel — Android // runtime permissions that can't be meaningfully exercised on the test host. - /// Returns true if the app has MANAGE_EXTERNAL_STORAGE. + /// Whether MANAGE_EXTERNAL_STORAGE is already held. + /// + /// A pure query: unlike [requestStoragePermission] this never navigates the + /// user anywhere, so startup can check it and move on. + /// + /// Answers "not granted" instead of throwing when there is no platform + /// channel to ask — the state on the `flutter test` host, where a + /// MissingPluginException from a widget's `initState` fails the test rather + /// than surfacing anything useful. "No permission" is also the truthful + /// answer there: the test host has no `/sdcard` to write to. Future hasStoragePermission() async { - return Permission.manageExternalStorage.isGranted; + try { + return await Permission.manageExternalStorage.isGranted; + } on MissingPluginException catch (error) { + debugPrint( + 'WorkoutApp: no permission_handler platform channel ($error) — ' + 'treating storage as not granted. Expected on the test host.', + ); + return false; + } } /// Requests MANAGE_EXTERNAL_STORAGE; opens the system settings page. /// + /// Only ever called from an explicit user action in Settings. Startup must + /// NOT call this: `Permission.manageExternalStorage.request()` throws the + /// user into a system Settings page whenever the permission is absent, and + /// since Firebase carries progression the `/sdcard` backup is a convenience, + /// not a requirement. Prompting for it on every launch made an optional + /// permission feel mandatory. + /// /// Returns true once granted. Future requestStoragePermission() async { final status = await Permission.manageExternalStorage.request(); @@ -161,8 +186,12 @@ class BackupService { dir.createSync(recursive: true); } await f.writeAsString(token); - } on Exception { + } on Exception catch (error) { // Backup is best-effort; never throw. + debugPrint( + 'WorkoutApp: could not mirror the sync token to $_syncTokenPath ' + '($error) — the token will NOT survive the next uninstall.', + ); } } @@ -183,8 +212,13 @@ class BackupService { dir.createSync(recursive: true); } await f.writeAsString(jsonEncode(data)); - } on Exception { + } on Exception catch (error) { // Backup is best-effort; never throw mid-workout. + debugPrint( + 'WorkoutApp: could not mirror the active session to ' + '$_activeSessionPath ($error) — an app-data wipe mid-workout would ' + 'now lose the set you are standing on.', + ); } } @@ -195,7 +229,12 @@ class BackupService { if (!f.existsSync()) return null; final decoded = jsonDecode(await f.readAsString()); return decoded is Map ? decoded : null; - } on Exception { + } on Exception catch (error) { + debugPrint( + 'WorkoutApp: mirrored active session at $_activeSessionPath exists ' + 'but is unreadable ($error) — an in-progress workout cannot be ' + 'resumed from it.', + ); return null; } } @@ -210,7 +249,11 @@ class BackupService { // coverage:ignore-start // Defensive: a present, readable token file is the norm; a portable way // to make readAsString throw here (unreadable file) isn't available. - } on Exception { + } on Exception catch (error) { + debugPrint( + 'WorkoutApp: mirrored sync token at $_syncTokenPath exists but is ' + 'unreadable ($error) — sync cannot be recovered from the backup.', + ); return null; } // coverage:ignore-end diff --git a/stronglift_replacement/workout_app/lib/services/firebase_backend.dart b/stronglift_replacement/workout_app/lib/services/firebase_backend.dart index a88b650..467aff1 100644 --- a/stronglift_replacement/workout_app/lib/services/firebase_backend.dart +++ b/stronglift_replacement/workout_app/lib/services/firebase_backend.dart @@ -12,6 +12,8 @@ /// half. On Android there is no such file. library; +import 'dart:developer'; + import 'package:crdt_sync/crdt_sync.dart'; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; @@ -50,9 +52,28 @@ Future loadAccount() async { return FirebaseAccount.tryParse( await _secure.read(key: kFirebaseAccountKey), ); - } on Exception { + } on Object catch (error) { // No secret service available: behave as "not configured" rather than - // crashing the settings screen. + // crashing the caller. Deliberately catches Object, not Exception: with no + // platform-channel binding (the `flutter test` host) this throws a + // FlutterError -- an Error, not an Exception -- which slipped straight + // through the old guard and failed any test whose code path reached the + // keystore. + // + // "Not configured" is the honest answer in both cases, and it is never + // silent: callers log their own "no Firebase account" reason, and this + // says which failure produced it. + // `log`, not `debugPrint`: debugPrint is stripped in release builds, and a + // keystore that has stopped answering means every sync path on this device + // silently reports "not configured" — the exact invisible failure the + // repo's CLAUDE.md forbids. + log( + 'WorkoutApp: cannot read the Firebase account from the keystore ' + '($error) — treating this device as NOT connected to sync, so ' + 'progression and workouts will not reach any other device.', + level: 1000, + error: error, + ); return null; } } diff --git a/stronglift_replacement/workout_app/lib/services/github_device_auth.dart b/stronglift_replacement/workout_app/lib/services/github_device_auth.dart index 6111d3a..2af9e9b 100644 --- a/stronglift_replacement/workout_app/lib/services/github_device_auth.dart +++ b/stronglift_replacement/workout_app/lib/services/github_device_auth.dart @@ -1,5 +1,6 @@ import 'dart:async'; import 'dart:convert'; +import 'package:flutter/foundation.dart' show debugPrint; import 'package:http/http.dart' as http; @@ -130,11 +131,15 @@ class GitHubDeviceAuth { }, ); json = jsonDecode(res.body) as Map; - } on Exception { + } on Exception catch (error) { // A dropped connection or malformed response is transient noise on a // long-lived poll loop (e.g. a flaky network) -- retry on the next // interval rather than aborting the whole ~15-minute authorization // window over one blip. The deadline above still bounds this. + debugPrint( + 'WorkoutApp: device-flow poll blipped ($error) — retrying on the ' + 'next interval until the authorization window closes.', + ); continue; } diff --git a/stronglift_replacement/workout_app/lib/services/http_server_service.dart b/stronglift_replacement/workout_app/lib/services/http_server_service.dart index 9eb7cc0..a8ed5f8 100644 --- a/stronglift_replacement/workout_app/lib/services/http_server_service.dart +++ b/stronglift_replacement/workout_app/lib/services/http_server_service.dart @@ -58,8 +58,13 @@ class HttpServerService { kWorkoutServerPort, ); unawaited(_serve()); - } on SocketException { + } on SocketException catch (error) { // Port already in use or binding failed — not fatal. + debugPrint( + 'WorkoutApp: could not bind the workout HTTP server on port ' + '$kWorkoutServerPort ($error) — the PC cannot pull workouts over ' + 'HTTP; ADB and sync still work.', + ); _server = null; } } @@ -78,8 +83,12 @@ class HttpServerService { try { final dir = await getExternalStorageDirectory(); if (dir != null) candidates.add('${dir.path}/workout_result.json'); - } on Exception { + } on Exception catch (error) { // Ignore; the /sdcard path is tried first. + debugPrint( + 'WorkoutApp: could not resolve the external storage directory ' + '($error) — only $kSyncFilePath will be tried.', + ); } // coverage:ignore-end } @@ -92,8 +101,12 @@ class HttpServerService { try { _latestJson = await file.readAsString(); return; - } on IOException { + } on IOException catch (error) { // Try next path. + debugPrint( + 'WorkoutApp: could not read $path ($error) — trying the next ' + 'candidate.', + ); } } // coverage:ignore-end diff --git a/stronglift_replacement/workout_app/lib/services/progression_sync_service.dart b/stronglift_replacement/workout_app/lib/services/progression_sync_service.dart new file mode 100644 index 0000000..aa90d23 --- /dev/null +++ b/stronglift_replacement/workout_app/lib/services/progression_sync_service.dart @@ -0,0 +1,433 @@ +/// Carries progression state and the in-progress session through Firebase. +/// +/// `backup.json` on external storage survives an uninstall, but only while the +/// user grants storage permission — deny it and a reinstall lands on factory +/// defaults with months of progression gone. This module gives the same data a +/// second, permission-free home so the app simply *has* its state after a +/// reinstall rather than needing a backfill. +/// +/// ## Why these paths sit OUTSIDE `devices/` +/// +/// Records live at `screen-locker-sync/exercise_state/` and +/// `screen-locker-sync/active_session`, as siblings of `devices/` rather than +/// inside it. Two independent reasons: +/// +/// * **Keyed on exercise name, not device.** A reinstall mints a new +/// per-install uuid ([currentSyncDeviceId]), so a per-device copy would be +/// orphaned by the very event this exists to survive. +/// * **Structurally invisible to the workout readers.** The PC's +/// `_workout_sync.py` and this app's [WorkoutSyncService] each list exactly +/// one prefix — `screen-locker-sync/devices` — and read `/log.json` +/// underneath it. Nothing lists the bare `screen-locker-sync` prefix, so a +/// progression record cannot be enumerated as a workout, let alone mistaken +/// for one. That matters because a progression record miscounted as a session +/// would silently hand out unlock credit for a workout nobody did. +library; + +import 'dart:convert'; +import 'dart:developer'; + +import 'package:crdt_sync/crdt_sync.dart'; +import 'package:workout_app/models/exercise.dart'; +import 'package:workout_app/models/workout_plan.dart'; +import 'package:workout_app/services/firebase_backend.dart'; +import 'package:workout_app/services/storage_service.dart'; +import 'package:workout_app/services/sync_device_id.dart'; + +/// Root for everything this module owns. Deliberately NOT under `devices/`. +const kProgressionPrefix = 'screen-locker-sync/exercise_state'; + +/// Path of the single shared in-progress session. +const kActiveSessionPath = 'screen-locker-sync/active_session'; + +const _payloadField = 'payload'; + +/// The outcome of a progression sync tick. +/// +/// Mirrors `WorkoutSyncService.PushResult`: never throw into the workout flow, +/// but never fail silently either — [reason] should read like a sentence a +/// human can act on. +class ProgressionSyncResult { + /// Creates a result. + const ProgressionSyncResult({ + required this.changed, + required this.reason, + this.count = 0, + }); + + /// Whether anything was actually written or applied. + final bool changed; + + /// How many exercise records were involved. + final int count; + + /// Why it did or did not happen. + final String reason; + + @override + String toString() => + 'ProgressionSyncResult(changed: $changed, count: $count, ' + 'reason: $reason)'; +} + +/// Pushes and pulls progression state + the active session over Firebase. +class ProgressionSyncService { + /// Creates a service. [firebaseFactory] is injected so tests can supply a + /// fake without reaching the OS keystore. + ProgressionSyncService({this.firebaseFactory}); + + /// Builds the Firebase backend, or null when this device is not set up. + final Future Function()? firebaseFactory; + + Future _openFirebase() => + (firebaseFactory ?? openFirebase)(); + + /// Every exercise across both plans, by name. + static Map get _defaults => { + for (final ex in [...workoutA, ...workoutB]) ex.name: ex, + }; + + static Map _stateToJson(ExerciseState s) => { + 'name': s.name, + 'weight': s.weight, + 'reps': s.reps, + 'success_streak': s.successStreak, + 'fail_streak': s.failStreak, + 'max_weight': s.maxWeight, + 'success_threshold': s.successThreshold, + 'fail_threshold': s.failThreshold, + }; + + static ExerciseState _stateFromJson(Map j) => ExerciseState( + name: j['name']! as String, + weight: (j['weight']! as num).toDouble(), + reps: (j['reps']! as num).toInt(), + successStreak: (j['success_streak'] as num?)?.toInt() ?? 0, + failStreak: (j['fail_streak'] as num?)?.toInt() ?? 0, + maxWeight: (j['max_weight']! as num).toDouble(), + successThreshold: (j['success_threshold'] as num?)?.toInt() ?? 3, + failThreshold: (j['fail_threshold'] as num?)?.toInt() ?? 2, + ); + + /// The remote path holding [name]'s progression record. + /// + /// The name is used raw on purpose: [FirebaseRestClient] escapes every path + /// segment with `encodeKey` before building the URI, so the characters + /// Realtime Database forbids in a key (`.`, `#`, `$`, `[`, `]`) are already + /// handled. Encoding here as well would double-escape and write to a + /// different key than [listDirectory] reports. + static String pathForExercise(String name) => '$kProgressionPrefix/$name'; + + /// Whether Firebase already holds a progression record for ANY of [states]. + /// + /// Any single one is enough to prove this account has real progression + /// stored, which is all the caller needs to decide not to overwrite it. + Future _remoteHasAnyProgression( + FirebaseRestClient client, + List states, + ) async { + for (final state in states) { + if (await _readRecord(client, pathForExercise(state.name)) != null) { + return true; + } + } + return false; + } + + /// Reads one remote record, returning its payload and HLC. + /// + /// Returns null when absent. A malformed record is reported at error level + /// and treated as absent rather than crashing the caller. + Future<(Map, Hlc)?> _readRecord( + FirebaseRestClient client, + String path, + ) async { + final text = await client.getFileText(path); + if (text == null) return null; + try { + final record = Record.fromJson( + jsonDecode(text) as Map, + ); + final field = record.fields[_payloadField]; + if (field == null) return null; + final payload = field.$1; + if (payload is! Map) return null; + return (payload.cast(), field.$2); + } on Object catch (error, stackTrace) { + log( + 'ProgressionSyncService: corrupt record at $path — treating as absent', + level: 1000, + error: error, + stackTrace: stackTrace, + ); + return null; + } + } + + Future _writeRecord( + FirebaseRestClient client, + String path, + String id, + Map payload, + Hlc hlc, + ) async { + final record = Record(id: id, fields: {_payloadField: (payload, hlc)}); + await client.putFileText( + path, + jsonEncode(record.toJson()), + // Ignored by the Realtime Database backend (no commit log), but the + // interface requires it and a GitHub-backed store would surface it. + message: 'progression: $id', + ); + } + + /// Pushes local progression to Firebase, one record per exercise. + /// + /// Each record carries an HLC derived from the remote one it replaces + /// (`previous:`), so the stamps form a causal chain per exercise rather than + /// depending on device clocks agreeing. + /// + /// Refuses to push at all from a freshly-installed database that still has + /// remote records to lose. Without that, the seed-then-push sequence a + /// reinstall performs would overwrite real progression with factory defaults + /// and leave Firebase as the only — wrong — copy. The test is + /// [StorageService.looksFreshlyInstalled], not a per-exercise value + /// comparison: a deliberate [StorageService.resetExerciseToDefaults] can + /// produce a row identical to a seeded one, and that reset must still sync. + Future pushProgression() async { + final client = await _openFirebase(); + if (client == null) { + const reason = + 'progression NOT pushed: no Firebase account on this device — ' + 'connect one in Settings, or progression lives only on this phone'; + log( + 'ProgressionSyncService.pushProgression skipped: $reason', + level: 900, + ); + return const ProgressionSyncResult(changed: false, reason: reason); + } + + try { + final states = await StorageService.instance.getAllExerciseStates(); + + // Decided ONCE, before any write. Evaluating this per exercise would + // still write every record Firebase happens not to hold yet before + // hitting the first one it does — a fresh install would leak factory + // defaults for exactly the exercises with no remote copy to protect them. + // + // Gated on hasSyncedProgression, NOT looksFreshlyInstalled: the only + // production caller finishes a workout (writing history and + // last_workout_type) before pushing, so a freshness test would already + // be false here and could never fire. + if (!await StorageService.instance.hasSyncedProgression() && + await _remoteHasAnyProgression(client, states)) { + const reason = + 'progression NOT pushed: this install has never pulled from ' + 'Firebase, but Firebase already holds progression. Refusing to ' + 'overwrite it with local state that may be factory defaults — ' + 'restart the app (or reconnect sync) so the real state is pulled ' + 'down first.'; + log('ProgressionSyncService: $reason', level: 1000); + return const ProgressionSyncResult(changed: false, reason: reason); + } + + var written = 0; + for (final state in states) { + final path = pathForExercise(state.name); + final remote = await _readRecord(client, path); + + await _writeRecord( + client, + path, + 'exercise_state:${state.name}', + _stateToJson(state), + // Chain from the record being replaced so the stamp is causal, not + // merely wall-clock: an offline device cannot mint a "newer" tick + // just by having a fast clock. + Hlc.newTick(currentSyncDeviceId, previous: remote?.$2), + ); + written++; + } + return ProgressionSyncResult( + changed: written > 0, + count: written, + reason: 'pushed $written exercise(s) to Firebase', + ); + } on Object catch (error, stackTrace) { + // Deliberately broad and swallowed: a sync failure must never cost the + // user their workout. Never silent, though — this logs at error level + // and the reason travels back to the caller. + final reason = 'progression push failed: $error'; + log( + 'ProgressionSyncService.pushProgression failed', + level: 1000, + error: error, + stackTrace: stackTrace, + ); + return ProgressionSyncResult(changed: false, reason: reason); + } finally { + client.close(); + } + } + + /// Pulls remote progression into the local DB. + /// + /// Applies only to a freshly-installed database + /// ([StorageService.looksFreshlyInstalled]) — so a pull can restore a wiped + /// install but can never clobber progression, or a deliberate reset, made on + /// this phone. That keeps the operation safe to run unconditionally at app + /// start. + Future pullProgression() async { + final client = await _openFirebase(); + if (client == null) { + const reason = + 'progression NOT pulled: no Firebase account on this device — ' + 'connect one in Settings to restore progression without storage ' + 'permission'; + log( + 'ProgressionSyncService.pullProgression skipped: $reason', + level: 900, + ); + return const ProgressionSyncResult(changed: false, reason: reason); + } + + try { + // Once this install has reconciled with Firebase, local state wins. + // Checked instead of looksFreshlyInstalled because a hand edit + // (setExerciseWeight, resetExerciseToDefaults) writes neither history + // nor last_workout_type: the DB still "looks fresh" afterwards, so a + // freshness test would silently revert the user's own change on the very + // next launch. + if (await StorageService.instance.hasSyncedProgression()) { + const reason = + 'progression NOT pulled: this install has already reconciled with ' + 'Firebase, so local state wins and must not be overwritten'; + log('ProgressionSyncService: $reason', level: 800); + return const ProgressionSyncResult(changed: false, reason: reason); + } + + var applied = 0; + for (final name in _defaults.keys) { + final remote = await _readRecord(client, pathForExercise(name)); + if (remote == null) continue; + await StorageService.instance.replaceExerciseState( + _stateFromJson(remote.$1), + ); + applied++; + } + // Marked even when nothing came down: reaching the backend and finding + // it empty IS a successful reconcile, and it is what unblocks this + // device's first push. Only a thrown error leaves the flag unset. + await StorageService.instance.markProgressionSynced(); + final reason = applied == 0 + ? 'no progression restored: Firebase holds no exercise records yet' + : 'restored $applied exercise(s) from Firebase'; + log('ProgressionSyncService.pullProgression: $reason', level: 800); + return ProgressionSyncResult( + changed: applied > 0, + count: applied, + reason: reason, + ); + } on Object catch (error, stackTrace) { + final reason = 'progression pull failed: $error'; + log( + 'ProgressionSyncService.pullProgression failed', + level: 1000, + error: error, + stackTrace: stackTrace, + ); + return ProgressionSyncResult(changed: false, reason: reason); + } finally { + client.close(); + } + } + + /// Publishes the in-progress session, or clears it when [data] is null. + /// + /// Called on set completion rather than per rep. `saveActiveSession` fires on + /// every tap, and a Firebase write per tap is exactly the traffic the sync + /// revision cache exists to avoid. + Future pushActiveSession( + Map? data, + ) async { + final client = await _openFirebase(); + if (client == null) { + const reason = 'active session NOT pushed: no Firebase account'; + log( + 'ProgressionSyncService.pushActiveSession skipped: $reason', + level: 900, + ); + return const ProgressionSyncResult(changed: false, reason: reason); + } + + try { + if (data == null) { + // An empty object, not a delete: the readers treat absent and empty + // alike, and a write keeps the HLC ordering intact for the next push. + await _writeRecord( + client, + kActiveSessionPath, + 'active_session', + const {}, + Hlc.newTick(currentSyncDeviceId), + ); + return const ProgressionSyncResult( + changed: true, + reason: 'cleared the shared active session', + ); + } + await _writeRecord( + client, + kActiveSessionPath, + 'active_session', + data, + Hlc.newTick(currentSyncDeviceId), + ); + return const ProgressionSyncResult( + changed: true, + reason: 'published the active session', + ); + } on Object catch (error, stackTrace) { + final reason = 'active session push failed: $error'; + log( + 'ProgressionSyncService.pushActiveSession failed', + level: 1000, + error: error, + stackTrace: stackTrace, + ); + return ProgressionSyncResult(changed: false, reason: reason); + } finally { + client.close(); + } + } + + /// Returns the shared in-progress session, or null when there is none. + /// + /// An empty payload means "explicitly cleared" and reads back as null, so a + /// finished workout is never resurrected. + Future?> readActiveSession() async { + final client = await _openFirebase(); + if (client == null) { + log( + 'ProgressionSyncService.readActiveSession skipped: no Firebase account', + level: 900, + ); + return null; + } + try { + final remote = await _readRecord(client, kActiveSessionPath); + if (remote == null || remote.$1.isEmpty) return null; + return remote.$1; + } on Object catch (error, stackTrace) { + log( + 'ProgressionSyncService.readActiveSession failed', + level: 1000, + error: error, + stackTrace: stackTrace, + ); + return null; + } finally { + client.close(); + } + } +} diff --git a/stronglift_replacement/workout_app/lib/services/storage_service.dart b/stronglift_replacement/workout_app/lib/services/storage_service.dart index 3959caa..a182471 100644 --- a/stronglift_replacement/workout_app/lib/services/storage_service.dart +++ b/stronglift_replacement/workout_app/lib/services/storage_service.dart @@ -9,6 +9,7 @@ import 'package:sqflite/sqflite.dart'; import 'package:workout_app/models/exercise.dart'; import 'package:workout_app/models/workout_plan.dart'; import 'package:workout_app/services/backup_service.dart'; +import 'package:workout_app/services/progression_sync_service.dart'; /// Per-exercise progression state stored in SQLite. class ExerciseState { @@ -81,8 +82,18 @@ class StorageService { static void resetForTesting({String dbPath = ':memory:'}) { _instance = null; _testDbPath = dbPath; + remoteActiveSessionReader = null; } + /// Reads the shared in-progress session from Firebase. Injected because the + /// real implementation reaches the OS keystore through a platform channel, + /// which `flutter test` has no binding for — and the resulting failure is an + /// `Error`, not an `Exception`, so it escapes the usual guards. + /// + /// Null means "use the real Firebase reader" (production). + @visibleForTesting + static Future?> Function()? remoteActiveSessionReader; + Future _open() async { final dbPath = _testDbPath ?? @@ -241,12 +252,20 @@ class StorageService { /// Falls back to the external mirror (re-seeding the table from it) when the /// table is empty — which is exactly the state right after an app-data wipe, /// so the workout resumes on the same set and rep instead of vanishing. + /// + /// Then, if the mirror is unavailable too, to Firebase. The mirror is tried + /// first because it is a local file read; Firebase costs a network round + /// trip, but it is the only copy that survives a reinstall with storage + /// permission denied — where `/sdcard` is unreadable by definition. Future?> loadActiveSession() async { final rows = await _db.query('active_session', where: 'id = 1'); if (rows.isNotEmpty) { return jsonDecode(rows.first['json']! as String) as Map; } - final mirrored = await BackupService.instance.readActiveSession(); + final mirrored = + await BackupService.instance.readActiveSession() ?? + await (remoteActiveSessionReader ?? + ProgressionSyncService().readActiveSession)(); if (mirrored == null) return null; await _db.insert( 'active_session', @@ -332,6 +351,30 @@ class StorageService { unawaited(_backupNow()); } + /// Overwrites [state] wholesale, inserting it when the row is absent. + /// + /// Used by the Firebase progression restore, which carries a complete record + /// (weight, reps, both streaks, max weight, both thresholds) and must land it + /// atomically — a field-by-field update would leave a half-restored row if it + /// failed partway. Unlike [setExerciseWeight] this deliberately does NOT + /// reset streaks: the streaks are part of what is being restored. + /// + /// Does not trigger a backup write. The caller is restoring INTO a + /// freshly-seeded database, and `_backupNow()` mid-restore is what previously + /// let default rows reach the only off-device copy. + Future replaceExerciseState(ExerciseState state) async { + await _db.insert('exercise_state', { + 'name': state.name, + 'weight': state.weight, + 'reps': state.reps, + 'success_streak': state.successStreak, + 'fail_streak': state.failStreak, + 'max_weight': state.maxWeight, + 'success_threshold': state.successThreshold, + 'fail_threshold': state.failThreshold, + }, conflictAlgorithm: ConflictAlgorithm.replace); + } + /// Sets the working weight for [name], resetting streaks. Future setExerciseWeight(String name, double weight) async { await _db.update( @@ -527,9 +570,13 @@ class StorageService { try { final decoded = jsonDecode(raw); return decoded is Map ? decoded : null; - } on FormatException { + } on FormatException catch (error) { // A corrupt local row must not block restoring the rest; it simply // cannot be matched against, so it is treated as "not seen". + debugPrint( + 'WorkoutApp: a stored session row is not valid JSON ($error) — ' + 'treating it as unseen, so a remote copy may be re-imported.', + ); return null; } } @@ -612,16 +659,56 @@ class StorageService { }); } - /// Restores from backup if the local DB is empty (fresh install). + /// Whether this database has never recorded a workout on this install. /// - /// "Empty" means no workout history and no [last_workout_type] setting. - Future restoreFromBackupIfNeeded() async { + /// "Empty" means no workout history and no [last_workout_type] setting — + /// the state a fresh install or a `pm clear` leaves behind, and the only + /// state in which overwriting local rows wholesale is safe. + /// + /// Shared with the Firebase progression restore so both restore paths agree + /// on what "fresh install" means. Deliberately NOT a per-exercise value + /// comparison: [resetExerciseToDefaults] leaves `reps` and `max_weight` + /// alone, so resetting an exercise that already sits at its default reps + /// produces a row byte-identical to a seeded one. Treating that as "fresh" + /// would let a pull silently revert a reset the user just performed. + Future looksFreshlyInstalled() async { final hasHistory = (await _db.rawQuery('SELECT COUNT(*) AS c FROM workout_history')) .first['c'] as int? ?? 0; - final hasType = await _getSetting('last_workout_type'); - if (hasHistory > 0 || hasType != null) return; // DB has real data + if (hasHistory > 0) return false; + return await _getSetting('last_workout_type') == null; + } + + /// Whether this install has reconciled its progression with Firebase. + /// + /// This — NOT [looksFreshlyInstalled] — is what gates pushing progression + /// up. The freshness test is unusable there because the only production + /// caller (`_finishWorkout`) writes a `workout_history` row and + /// `last_workout_type` *before* it pushes, so the DB has stopped looking + /// fresh by the time the guard runs: the guard could never fire, and a + /// reinstall that connected Firebase late would overwrite every real remote + /// record with factory defaults on its first finished workout. + /// + /// It is also wrong for hand edits: `setExerciseWeight` and + /// `resetExerciseToDefaults` write neither history nor `last_workout_type`, + /// so a DB still "looks fresh" after them and the next pull would revert the + /// user's own change. + Future hasSyncedProgression() async => + await _getSetting(_progressionSyncedKey) != null; + + /// Records that remote progression has been reconciled, so later pushes are + /// allowed and later pulls stop overwriting local state. + Future markProgressionSynced() => + _setSetting(_progressionSyncedKey, DateTime.now().toIso8601String()); + + static const _progressionSyncedKey = 'progression_synced_at'; + + /// Restores from backup if the local DB is empty (fresh install). + /// + /// "Empty" means no workout history and no [last_workout_type] setting. + Future restoreFromBackupIfNeeded() async { + if (!await looksFreshlyInstalled()) return; // DB has real data final backup = await BackupService.instance.readBackup(); if (backup == null) { diff --git a/stronglift_replacement/workout_app/lib/services/sync_service.dart b/stronglift_replacement/workout_app/lib/services/sync_service.dart index 774882b..82f6a3e 100644 --- a/stronglift_replacement/workout_app/lib/services/sync_service.dart +++ b/stronglift_replacement/workout_app/lib/services/sync_service.dart @@ -2,6 +2,7 @@ library; import 'dart:io'; +import 'package:flutter/foundation.dart' show debugPrint; import 'package:path_provider/path_provider.dart'; import 'package:workout_app/models/workout_session.dart'; import 'package:workout_app/services/http_server_service.dart'; @@ -25,8 +26,12 @@ class SyncService { final file = File(kSyncFilePath); await file.writeAsString(json); return const SyncResult(success: true, path: kSyncFilePath); - } on Exception { + } on Exception catch (error) { // Fallback: app-specific external directory (still ADB accessible). + debugPrint( + 'WorkoutApp: could not write $kSyncFilePath ($error) — trying the ' + 'app-specific external directory next.', + ); } // External storage is Android-only; getExternalStorageDirectory() throws @@ -42,8 +47,12 @@ class SyncService { await file.writeAsString(json); return SyncResult(success: true, path: file.path); } - } on Exception { + } on Exception catch (error) { // Fallback failed. + debugPrint( + 'WorkoutApp: the external-storage fallback failed too ($error) — ' + 'this workout will NOT be readable over ADB/HTTP.', + ); } } // coverage:ignore-end diff --git a/stronglift_replacement/workout_app/lib/services/sync_settings.dart b/stronglift_replacement/workout_app/lib/services/sync_settings.dart index 8f14039..ef2b4a0 100644 --- a/stronglift_replacement/workout_app/lib/services/sync_settings.dart +++ b/stronglift_replacement/workout_app/lib/services/sync_settings.dart @@ -8,6 +8,7 @@ /// library doc for the security trade-off this represents. library; +import 'package:flutter/foundation.dart' show debugPrint; import 'package:flutter/services.dart' show PlatformException; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:workout_app/services/backup_service.dart'; @@ -53,7 +54,11 @@ class SyncSettings { String? token; try { token = await _secure.read(key: _secureToken); - } on PlatformException { + } on PlatformException catch (error) { + debugPrint( + 'WorkoutApp: could not read the sync token from the keystore ' + '($error) — falling back to the external backup below.', + ); token = null; } if (token == null || token.isEmpty) { @@ -62,8 +67,13 @@ class SyncSettings { token = backedUp; try { await _secure.write(key: _secureToken, value: backedUp); - } on PlatformException { + } on PlatformException catch (error) { // Keystore unavailable; still return the recovered token below. + debugPrint( + 'WorkoutApp: recovered the sync token from the backup but could ' + 'NOT write it back to the keystore ($error) — sync works this ' + 'run, but every launch will keep re-reading the backup.', + ); } } } @@ -88,8 +98,13 @@ class SyncSettings { } try { await _secure.write(key: _secureToken, value: backedUp); - } on PlatformException { + } on PlatformException catch (error) { // Keystore unavailable — the caller can still use the returned token. + debugPrint( + 'WorkoutApp: could not re-seed the keystore with the recovered sync ' + 'token ($error) — this retry still works, but the stale entry will ' + 'keep shadowing the backup on future launches.', + ); } return backedUp; } @@ -106,7 +121,12 @@ class SyncSettings { await _secure.write(key: _secureToken, value: token); } return true; - } on PlatformException { + } on PlatformException catch (error) { + debugPrint( + 'WorkoutApp: could not save the sync token to the keystore ($error) ' + '— it is still mirrored to external storage, so sync survives, but ' + 'the keystore copy is missing.', + ); return false; } } diff --git a/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart b/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart index 802d5b2..b5961b3 100644 --- a/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart +++ b/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart @@ -16,6 +16,7 @@ import 'package:workout_app/services/backup_service.dart'; import 'package:workout_app/services/sync_settings.dart'; import 'package:workout_app/models/workout_plan.dart'; import 'package:workout_app/screens/settings_screen.dart'; +import 'package:workout_app/services/progression_sync_service.dart'; import 'package:workout_app/services/storage_service.dart'; import 'package:workout_app/ui/theme.dart'; @@ -67,6 +68,9 @@ void main() { Future Function()? accountLoader, Future Function(FirebaseAccount)? accountSaver, Future Function()? accountClearer, + Future Function()? storageChecker, + Future Function()? storageRequester, + Future Function()? progressionPuller, }) => MaterialApp( theme: buildAppTheme(), home: SettingsScreen( @@ -77,6 +81,17 @@ void main() { accountLoader: accountLoader ?? () async => null, accountSaver: accountSaver, accountClearer: accountClearer, + // Same reason: permission_handler is a platform channel too. + storageChecker: storageChecker ?? () async => false, + storageRequester: storageRequester, + // Default to a no-op pull: connecting triggers one, and the real service + // would open a database and hit the network from a widget test. + progressionPuller: + progressionPuller ?? + () async => const ProgressionSyncResult( + changed: false, + reason: 'stubbed in tests', + ), ), ); @@ -314,6 +329,49 @@ void main() { expect(find.text('sync@example.com'), findsOneWidget); }); + testWidgets('connecting pulls progression down immediately', (tester) async { + // Regression: connecting is the NORMAL path after a reinstall, because the + // uninstall wipes the keystore and startup's pull therefore found no + // account. Without pulling here the device sits on factory defaults while + // holding real remote progression, and its first finished workout pushes + // those defaults over the top. + var pulled = false; + await _pump( + tester, + _wrap( + firebaseFactory: () async => _stubFirebaseClient(), + progressionPuller: () async { + pulled = true; + return const ProgressionSyncResult( + changed: true, + count: 7, + reason: 'restored 7 exercise(s) from Firebase', + ); + }, + ), + ); + await _scrollToGitHubSync(tester); + + await tester.enterText( + find.widgetWithText(TextField, 'Sync account email'), + 'sync@example.com', + ); + await tester.enterText( + find.widgetWithText(TextField, 'Sync account password'), + 'pw', + ); + await tester.ensureVisible(find.text('Connect Firebase')); + await tester.pumpAndSettle(); + await tester.tap(find.text('Connect Firebase')); + await tester.pumpAndSettle(); + + expect(pulled, isTrue); + expect( + find.text('Connected. Restored 7 exercise(s) from Firebase.'), + findsOneWidget, + ); + }); + testWidgets('a rejected account is cleared rather than left half-stored', ( tester, ) async { @@ -374,6 +432,54 @@ void main() { expect(find.text('Firebase disconnected.'), findsOneWidget); }); + testWidgets('offline backup offers a grant button when not held', ( + tester, + ) async { + var asked = false; + await _pump( + tester, + _wrap( + storageChecker: () async => false, + storageRequester: () async { + asked = true; + return true; + }, + ), + ); + await tester.scrollUntilVisible( + find.text('Grant storage permission'), + 500, + scrollable: find.byType(Scrollable).first, + ); + // Framed as optional: Firebase restores progression without it. + expect( + find.textContaining('Optional.', skipOffstage: false), + findsOneWidget, + ); + + await tester.tap(find.text('Grant storage permission')); + await tester.pumpAndSettle(); + + expect(asked, isTrue, reason: 'the grant page is opened on demand only'); + expect(find.text('Storage permission granted'), findsOneWidget); + expect(find.text('Grant storage permission'), findsNothing); + }); + + testWidgets('offline backup reports an already-granted permission', ( + tester, + ) async { + await _pump(tester, _wrap(storageChecker: () async => true)); + await tester.scrollUntilVisible( + find.text('Storage permission granted'), + 500, + scrollable: find.byType(Scrollable).first, + ); + + expect(find.text('Storage permission granted'), findsOneWidget); + expect(find.text('Grant storage permission'), findsNothing); + expect(find.textContaining('Granted.', skipOffstage: false), findsOneWidget); + }); + testWidgets('leads with Firebase and hides GitHub behind Advanced', ( tester, ) async { @@ -381,8 +487,17 @@ void main() { await _scrollToGitHubSync(tester); // GitHub is the cutover mirror, not a peer choice. - expect(find.text('SYNC'), findsOneWidget); - expect(find.text('Connect Firebase'), findsOneWidget); + // + // `SYNC` is found with a scroll-independent finder: the OFFLINE BACKUP + // section added below `Advanced (GitHub mirror)` made the page taller, so + // scrolling that tile into view now pushes the SYNC header off the top. + // What this test asserts is the ORDER (Firebase leads, GitHub is hidden), + // not that both happen to fit on screen at once. + expect(find.text('SYNC', skipOffstage: false), findsOneWidget); + expect( + find.text('Connect Firebase', skipOffstage: false), + findsOneWidget, + ); expect(find.text('Connect GitHub'), findsNothing); await openAdvanced(tester); diff --git a/stronglift_replacement/workout_app/test/services/progression_sync_service_test.dart b/stronglift_replacement/workout_app/test/services/progression_sync_service_test.dart new file mode 100644 index 0000000..db990e4 --- /dev/null +++ b/stronglift_replacement/workout_app/test/services/progression_sync_service_test.dart @@ -0,0 +1,426 @@ +import 'dart:convert'; + +import 'package:crdt_sync/crdt_sync.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:sqflite_common_ffi/sqflite_ffi.dart'; +import 'package:workout_app/services/progression_sync_service.dart'; +import 'package:workout_app/services/storage_service.dart'; + +/// In-memory stand-in for [FirebaseRestClient]. +/// +/// Records every write so a test can assert not just the final state but that +/// no write happened at all -- the difference between "the guard held" and +/// "the guard wrote defaults and then wrote them back". +class _FakeStore implements FirebaseRestClient { + final Map files = {}; + int writes = 0; + bool closed = false; + Object? throwOnGet; + + @override + Future getFileText(String path) async { + if (throwOnGet != null) throw throwOnGet!; + return files[path]; + } + + @override + Future putFileText( + String path, + String text, { + required String message, + }) async { + writes++; + files[path] = text; + } + + @override + Future> listDirectory(String path) async => files.keys + .where((k) => k.startsWith('$path/')) + .map((k) => k.substring(path.length + 1).split('/').first) + .toSet() + .toList(); + + @override + void close() => closed = true; + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +/// Writes a progression record for [name] straight into [store]. +void _seedRemote( + _FakeStore store, + String name, { + double weight = 20, + int reps = 12, + double maxWeight = 27.5, +}) { + final record = Record( + id: 'exercise_state:$name', + fields: { + 'payload': ( + { + 'name': name, + 'weight': weight, + 'reps': reps, + 'success_streak': 0, + 'fail_streak': 0, + 'max_weight': maxWeight, + 'success_threshold': 3, + 'fail_threshold': 2, + }, + Hlc.newTick('remote-device'), + ), + }, + ); + store.files[ProgressionSyncService.pathForExercise(name)] = jsonEncode( + record.toJson(), + ); +} + +Hlc _hlcAt(_FakeStore store, String name) => Record.fromJson( + jsonDecode(store.files[ProgressionSyncService.pathForExercise(name)]!) + as Map, +).fields['payload']!.$2; + +void main() { + late _FakeStore store; + late ProgressionSyncService sync; + + setUpAll(() { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + }); + + setUp(() async { + store = _FakeStore(); + sync = ProgressionSyncService(firebaseFactory: () async => store); + StorageService.resetForTesting(); + await StorageService.init(); + }); + + /// Marks this install as having reconciled with Firebase, which is what + /// unblocks pushing. + Future markSynced() => + StorageService.instance.markProgressionSynced(); + + group('paths', () { + test('progression sits outside devices/, so workout readers cannot see it', () { + // The whole safety argument: the PC lists `screen-locker-sync/devices` + // and reads `/log.json` under it. A progression record that never + // appears under that prefix cannot be miscounted as a workout, which + // would silently grant unlock credit. + final path = ProgressionSyncService.pathForExercise('Situp'); + expect(path, startsWith('screen-locker-sync/exercise_state/')); + expect(path, isNot(contains('/devices/'))); + expect(kActiveSessionPath, isNot(contains('/devices/'))); + }); + + test('the exercise name is used raw, so the client can escape it once', () { + // Double-encoding here would write to a different key than + // listDirectory reports back. + expect( + ProgressionSyncService.pathForExercise('Dumbbell Bench Press'), + endsWith('/Dumbbell Bench Press'), + ); + }); + }); + + group('pullProgression', () { + test('restores every remote record into a fresh install', () async { + _seedRemote(store, 'Dumbbell Bench Press', weight: 20); + _seedRemote(store, 'Situp', weight: 10, reps: 31, maxWeight: 10); + + final result = await sync.pullProgression(); + + expect(result.changed, isTrue); + expect(result.count, 2); + final bench = await StorageService.instance.getExerciseState( + 'Dumbbell Bench Press', + ); + expect(bench!.weight, 20, reason: 'restored, not the factory 22.5'); + final situp = await StorageService.instance.getExerciseState('Situp'); + expect(situp!.reps, 31, reason: 'restored, not the factory 30'); + }); + + test('declines once this install has reconciled with Firebase', () async { + _seedRemote(store, 'Dumbbell Bench Press', weight: 20); + await markSynced(); + await StorageService.instance.setExerciseWeight( + 'Dumbbell Bench Press', + 42.5, + ); + + final result = await sync.pullProgression(); + + expect(result.changed, isFalse); + expect(result.reason, contains('already reconciled')); + final bench = await StorageService.instance.getExerciseState( + 'Dumbbell Bench Press', + ); + expect(bench!.weight, 42.5, reason: 'local progression survived'); + }); + + test('a hand-edited weight is NOT reverted on the next launch', () async { + // Regression: gating the pull on looksFreshlyInstalled() reverted this. + // setExerciseWeight writes neither history nor last_workout_type, so the + // DB still "looks fresh" afterwards and the next launch pulled the old + // remote value back over the user's own edit. + _seedRemote(store, 'Dumbbell Bench Press', weight: 60); + await sync.pullProgression(); // first launch reconciles + await StorageService.instance.setExerciseWeight( + 'Dumbbell Bench Press', + 30, + ); + + await sync.pullProgression(); // next launch + + final bench = await StorageService.instance.getExerciseState( + 'Dumbbell Bench Press', + ); + expect(bench!.weight, 30, reason: 'the user edit stands'); + }); + + test('a successful pull unblocks this device for pushing', () async { + expect(await StorageService.instance.hasSyncedProgression(), isFalse); + await sync.pullProgression(); + expect( + await StorageService.instance.hasSyncedProgression(), + isTrue, + reason: 'an empty backend is still a successful reconcile', + ); + }); + + test('a failed pull leaves this device blocked from pushing', () async { + store.throwOnGet = FirebaseSyncError('rtdb exploded'); + await sync.pullProgression(); + expect( + await StorageService.instance.hasSyncedProgression(), + isFalse, + reason: 'never reconciled, so a later push must not clobber', + ); + }); + + test('reports the empty case rather than claiming success', () async { + final result = await sync.pullProgression(); + expect(result.changed, isFalse); + expect(result.reason, contains('no exercise records')); + }); + + test('says why when no Firebase account is configured', () async { + final offline = ProgressionSyncService(firebaseFactory: () async => null); + final result = await offline.pullProgression(); + expect(result.changed, isFalse); + expect(result.reason, contains('no Firebase account')); + }); + + test('reports a backend failure instead of swallowing it', () async { + store.throwOnGet = FirebaseSyncError('rtdb exploded'); + final result = await sync.pullProgression(); + expect(result.changed, isFalse); + expect(result.reason, contains('pull failed')); + expect(store.closed, isTrue); + }); + + test('treats a corrupt remote record as absent', () async { + store.files[ProgressionSyncService.pathForExercise('Situp')] = + 'not json at all'; + final result = await sync.pullProgression(); + expect(result.changed, isFalse); + }); + }); + + group('pushProgression', () { + test('writes every exercise from a real install', () async { + await markSynced(); + final result = await sync.pushProgression(); + + expect(result.changed, isTrue); + expect(result.count, 7); + expect(store.files.keys, contains(endsWith('/Situp'))); + }); + + test( + 'a fresh install writes NOTHING when Firebase already holds progression', + () async { + // The clobber guard. Evaluated once, before any write: a per-exercise + // check still leaks factory defaults for every exercise Firebase + // happens not to hold yet -- exactly the ones with no copy to lose. + _seedRemote(store, 'Dumbbell Bench Press', weight: 20); + final before = Map.from(store.files); + + final result = await sync.pushProgression(); + + expect(result.changed, isFalse); + expect(result.reason, contains('never pulled from Firebase')); + expect(store.writes, 0, reason: 'not one write, not even a partial'); + expect(store.files, before); + expect( + store.files.containsKey( + ProgressionSyncService.pathForExercise('Dumbbell Lunge'), + ), + isFalse, + reason: 'an un-backed exercise must not gain a default record', + ); + }, + ); + + test( + 'finishing a workout on a never-pulled install cannot clobber remote', + () async { + // The exact production sequence that made the old guard useless: + // _finishWorkout writes a workout_history row AND last_workout_type + // BEFORE it pushes, so looksFreshlyInstalled() was already false by + // the time the guard ran and factory defaults went up regardless. + // Reachable without a fresh install too: connect Firebase late (the + // uninstall wipes the keystore, so startup's pull found no account). + _seedRemote(store, 'Dumbbell Bench Press', weight: 60); + await StorageService.instance.saveSession( + date: '2026-08-10', + workoutType: 'A', + durationSeconds: 60, + succeeded: true, + json: '{}', + ); + await StorageService.instance.setLastWorkoutType('A'); + expect( + await StorageService.instance.looksFreshlyInstalled(), + isFalse, + reason: 'this is why the freshness test could not protect anything', + ); + + final result = await sync.pushProgression(); + + expect(result.changed, isFalse); + expect(store.writes, 0); + expect( + store.files[ProgressionSyncService.pathForExercise( + 'Dumbbell Bench Press', + )], + contains('60'), + reason: 'the real remote progression is untouched', + ); + }, + ); + + test('a fresh install with an empty backend still seeds it', () async { + final result = await sync.pushProgression(); + expect(result.changed, isTrue, reason: 'nothing to clobber'); + }); + + test('a deliberate reset still reaches Firebase', () async { + // resetExerciseToDefaults leaves reps and max_weight alone, so resetting + // an exercise already at its default reps produces a row identical to a + // seeded one. A value-based guard would refuse to push it and then + // revert it on the next launch. + _seedRemote(store, 'Dumbbell Bench Press', weight: 20); + await markSynced(); + await StorageService.instance.resetExerciseToDefaults( + 'Dumbbell Bench Press', + ); + + final result = await sync.pushProgression(); + + expect(result.changed, isTrue); + expect( + store.files[ProgressionSyncService.pathForExercise( + 'Dumbbell Bench Press', + )], + contains('22.5'), + ); + }); + + test('stamps chain causally per record', () async { + await markSynced(); + await sync.pushProgression(); + final first = _hlcAt(store, 'Situp'); + await sync.pushProgression(); + final second = _hlcAt(store, 'Situp'); + + expect(first < second, isTrue); + }); + + test('says why when no Firebase account is configured', () async { + final offline = ProgressionSyncService(firebaseFactory: () async => null); + final result = await offline.pushProgression(); + expect(result.changed, isFalse); + expect(result.reason, contains('no Firebase account')); + }); + + test('reports a backend failure instead of swallowing it', () async { + await markSynced(); + store.throwOnGet = FirebaseSyncError('rtdb exploded'); + final result = await sync.pushProgression(); + expect(result.changed, isFalse); + expect(result.reason, contains('push failed')); + }); + }); + + group('active session', () { + test('round-trips through Firebase', () async { + expect(await sync.readActiveSession(), isNull); + + await sync.pushActiveSession({'workoutType': 'B', 'startTimeMs': 123}); + + final read = await sync.readActiveSession(); + expect(read!['workoutType'], 'B'); + }); + + test('clearing it cannot resurrect a finished workout', () async { + await sync.pushActiveSession({'workoutType': 'A'}); + final result = await sync.pushActiveSession(null); + + expect(result.changed, isTrue); + expect(result.reason, contains('cleared')); + expect(await sync.readActiveSession(), isNull); + }); + + test('says why when no Firebase account is configured', () async { + final offline = ProgressionSyncService(firebaseFactory: () async => null); + final result = await offline.pushActiveSession({'a': 1}); + expect(result.changed, isFalse); + expect(result.reason, contains('no Firebase account')); + expect(await offline.readActiveSession(), isNull); + }); + + test('reports a push failure instead of swallowing it', () async { + store.throwOnGet = FirebaseSyncError('rtdb exploded'); + final result = await sync.pushActiveSession({'a': 1}); + // The write path does not read first, so this one succeeds; the read + // path is what surfaces the error. + expect(result.changed, isTrue); + expect(await sync.readActiveSession(), isNull); + }); + + test('a failed write is reported, not swallowed', () async { + final result = await ProgressionSyncService( + firebaseFactory: () async => _ExplodingStore(), + ).pushActiveSession({'a': 1}); + + expect(result.changed, isFalse); + expect(result.reason, contains('active session push failed')); + }); + }); + + test('ProgressionSyncResult prints its fields for debugging', () { + const result = ProgressionSyncResult( + changed: true, + count: 3, + reason: 'pushed', + ); + expect( + result.toString(), + 'ProgressionSyncResult(changed: true, count: 3, reason: pushed)', + ); + }); +} + +/// A store whose writes always fail, for the push-error paths. +class _ExplodingStore extends _FakeStore { + @override + Future putFileText( + String path, + String text, { + required String message, + }) async => throw FirebaseSyncError('write refused'); +}