diff --git a/install_systemd.sh b/install_systemd.sh index ef0f002..86b2697 100755 --- a/install_systemd.sh +++ b/install_systemd.sh @@ -4,9 +4,13 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" SERVICE_FILE="$SCRIPT_DIR/workout-locker.service" EARLY_BIRD_TIMER_FILE="$SCRIPT_DIR/early-bird-workout-check.timer" +SYNC_SERVICE_FILE="$SCRIPT_DIR/workout-sync.service" +SYNC_TIMER_FILE="$SCRIPT_DIR/workout-sync.timer" USER_SERVICE_DIR="$HOME/.config/systemd/user" SERVICE_NAME="workout-locker.service" EARLY_BIRD_TIMER_NAME="early-bird-workout-check.timer" +SYNC_SERVICE_NAME="workout-sync.service" +SYNC_TIMER_NAME="workout-sync.timer" # Check if service is already installed if [ -f "$USER_SERVICE_DIR/$SERVICE_NAME" ]; then @@ -44,6 +48,12 @@ cp "$SERVICE_FILE" "$USER_SERVICE_DIR/$SERVICE_NAME" # Copy early bird timer cp "$EARLY_BIRD_TIMER_FILE" "$USER_SERVICE_DIR/$EARLY_BIRD_TIMER_NAME" +# Copy the periodic workout-sync units. Without these, syncing happens only +# once per locker start, so a workout finished after login is not seen until +# the next login. +cp "$SYNC_SERVICE_FILE" "$USER_SERVICE_DIR/$SYNC_SERVICE_NAME" +cp "$SYNC_TIMER_FILE" "$USER_SERVICE_DIR/$SYNC_TIMER_NAME" + # Update paths in the service file to use absolute paths REPO_ROOT="$SCRIPT_DIR" sed -i "s|WorkingDirectory=.*|WorkingDirectory=$REPO_ROOT|" "$USER_SERVICE_DIR/$SERVICE_NAME" @@ -59,6 +69,9 @@ systemctl --user enable "$SERVICE_NAME" # Enable the early bird re-check timer systemctl --user enable --now "$EARLY_BIRD_TIMER_NAME" +# Enable the periodic workout sync +systemctl --user enable --now "$SYNC_TIMER_NAME" + echo "✓ Workout locker service installed" echo "✓ Early bird re-check timer installed (fires daily at 08:30)" echo "✓ Service will start automatically on next login" @@ -81,6 +94,11 @@ if systemctl --user is-enabled "$EARLY_BIRD_TIMER_NAME" &>/dev/null; then else echo "✗ early bird timer: NOT enabled" fi +if systemctl --user is-enabled "$SYNC_TIMER_NAME" &>/dev/null; then + echo "✓ workout sync timer: INSTALLED and enabled (every 15 min)" +else + echo "✗ workout sync timer: NOT enabled" +fi I3_CONFIG="$HOME/.config/i3/config" if [ -f "$I3_CONFIG" ] && grep -q "exec.*screen_lock.py" "$I3_CONFIG"; then diff --git a/pyproject.toml b/pyproject.toml index dca6e18..749b21c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ requires-python = ">=3.10" # also used by diet_guard; here for phone-workout GitHub sync (Workstream C) dependencies = [ "gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock", - "crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync", + "crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.7.0#subdirectory=crdt-sync", ] [project.scripts] diff --git a/remove_systemd.sh b/remove_systemd.sh index 438e450..6673a3f 100755 --- a/remove_systemd.sh +++ b/remove_systemd.sh @@ -5,19 +5,23 @@ SERVICE_NAME="workout-locker.service" TIMER_NAME="workout-locker.timer" USER_SERVICE_DIR="$HOME/.config/systemd/user" -# Stop the service and timer if running -systemctl --user stop "$TIMER_NAME" 2>/dev/null -systemctl --user stop "$SERVICE_NAME" 2>/dev/null +# Every unit install_systemd.sh creates. Removal has to enumerate all of them, +# or a "removed" locker keeps firing from a timer nobody remembers installing. +UNITS=( + "$SERVICE_NAME" + "$TIMER_NAME" + "early-bird-workout-check.timer" + "workout-sync.timer" + "workout-sync.service" +) -# Disable the service and timer -systemctl --user disable "$TIMER_NAME" 2>/dev/null -systemctl --user disable "$SERVICE_NAME" 2>/dev/null - -# Remove service and timer files -rm -f "$USER_SERVICE_DIR/$SERVICE_NAME" -rm -f "$USER_SERVICE_DIR/$TIMER_NAME" +for unit in "${UNITS[@]}"; do + systemctl --user stop "$unit" 2>/dev/null + systemctl --user disable "$unit" 2>/dev/null + rm -f "$USER_SERVICE_DIR/$unit" +done # Reload systemd daemon systemctl --user daemon-reload -echo "✓ Workout locker service and timer removed" +echo "✓ Workout locker services and timers removed" diff --git a/requirements.txt b/requirements.txt index e171cdd..ff81626 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ bandit>=1.7.0 codespell>=2.2.0 coverage>=7.4.0 -crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync +crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.7.0#subdirectory=crdt-sync gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock mcp>=2,<3 mypy>=1.8.0 diff --git a/screen_locker/_sync_retry.py b/screen_locker/_sync_retry.py index dc4ab02..5df6708 100644 --- a/screen_locker/_sync_retry.py +++ b/screen_locker/_sync_retry.py @@ -11,8 +11,13 @@ Retrying at the application layer rather than ordering the unit after ordering does not apply, and the user services involved cannot depend on a system target anyway. -Any :class:`GitHubSyncError` is retried, deliberately including -:class:`RepoNotFoundError`. Classifying "transient" vs "permanent" would mean +Any :class:`RemoteSyncError` is retried, deliberately including +:class:`RepoNotFoundError`. Catching the common parent rather than +:class:`GitHubSyncError` matters since the Firebase cutover: +:class:`FirebaseSyncError` is a *sibling* of :class:`GitHubSyncError`, not a +subclass, so catching the latter let every Firebase failure escape uncaught and +take the whole locker down (``status=1/FAILURE``) instead of degrading to "no +synced workout". Classifying "transient" vs "permanent" would mean depending on which exception ``crdt_sync`` chains where, which is internal to that library and has changed before; a few wasted seconds on a genuinely misconfigured token is a much cheaper mistake than skipping a real retry. @@ -24,7 +29,7 @@ import logging from time import sleep from typing import TYPE_CHECKING, TypeVar -from crdt_sync import GitHubSyncError +from crdt_sync import RemoteSyncError from screen_locker._constants import ( SYNC_RETRY_ATTEMPTS, @@ -60,7 +65,7 @@ def with_sync_retry( Whatever *operation* returns. Raises: - GitHubSyncError: If every attempt failed. The caller keeps its own + RemoteSyncError: If every attempt failed. The caller keeps its own handling — this only buys time, it never hides the outcome. """ last_attempt = max(attempts, 1) @@ -69,7 +74,7 @@ def with_sync_retry( for attempt in range(1, last_attempt): try: return operation() - except GitHubSyncError as exc: + except RemoteSyncError as exc: wait = delay_seconds * (2 ** (attempt - 1)) _logger.warning( "Could not %s (attempt %d/%d): %s — retrying in %.0fs; " @@ -85,7 +90,7 @@ def with_sync_retry( try: return operation() - except GitHubSyncError as exc: + except RemoteSyncError as exc: _logger.warning( "Could not %s after %d attempt(s): %s — giving up", description, diff --git a/screen_locker/_workout_sync.py b/screen_locker/_workout_sync.py index 6a3ac0f..19253f6 100644 --- a/screen_locker/_workout_sync.py +++ b/screen_locker/_workout_sync.py @@ -12,22 +12,22 @@ from __future__ import annotations import json import logging +from typing import TYPE_CHECKING from crdt_sync import ( CONFIG_FILE, ConfigError, FirebaseAuthError, GitHubSyncClient, - GitHubSyncError, Hlc, Record, RemoteStore, RemoteSyncError, + firebase_client_for, mirror_client_for, ) from screen_locker._constants import ( - SYNC_PHONE_DEVICE_ID, SYNC_REPO_NAME, SYNC_REPO_OWNER, SYNC_TIMEOUT_SECONDS, @@ -36,10 +36,12 @@ from screen_locker._constants import ( from screen_locker._manual_workout import MANUAL_WORKOUT_SYNC_KIND from screen_locker._sync_retry import with_sync_retry +if TYPE_CHECKING: + from collections.abc import Callable + _logger = logging.getLogger(__name__) _DEVICES_PREFIX = "screen-locker-sync/devices" -_LOG_PATH = f"{_DEVICES_PREFIX}/{SYNC_PHONE_DEVICE_ID}/log.json" _PAYLOAD_FIELD = "payload" @@ -48,6 +50,22 @@ def _is_manual_payload(payload: object) -> bool: return isinstance(payload, dict) and payload.get("kind") == MANUAL_WORKOUT_SYNC_KIND +def _is_session_payload(payload: object) -> bool: + """True if a decoded sync payload is a completed StrongLifts session. + + Identified by SHAPE -- the presence of ``exercises`` -- not by a ``kind`` + field, because the phone app's ``WorkoutSession.toJson()`` emits no ``kind`` + at all. Keying session detection on ``kind`` is what made real sessions + invisible to the only reader that walks every device directory. + + Shape-matching also excludes the PC-origin ``runnerup_verified`` / + ``phone_verified`` records that share these device logs: they are non-manual + too, so "not manual" alone would hand a caller expecting ``exercises`` and + ``duration_seconds`` a record that has neither. + """ + return isinstance(payload, dict) and isinstance(payload.get("exercises"), list) + + def read_sync_token() -> str | None: """Return the saved sync PAT, or None if sync isn't configured. @@ -61,38 +79,81 @@ def read_sync_token() -> str | None: return token or None -def _latest_payload(log_json: str) -> dict | None: - """Return the most recently pushed workout payload from a log.json blob. +def _merge_device_records( + client: RemoteStore, + extract: Callable[[str], dict[str, tuple[dict, Hlc]]], + what: str, + *, + strict: bool = False, +) -> dict[str, tuple[dict, Hlc]]: + """Merge ``extract``-selected records across EVERY device log. - Raises: - TypeError: If the top-level JSON isn't an object, a record's shape - doesn't match what :meth:`Record.from_dict` expects, or the - payload field itself isn't a JSON object. - KeyError: If a record is missing an expected key. - ValueError: Via ``json.loads``, if the text isn't valid JSON, or via - :meth:`crdt_sync.Hlc.from_str` on a malformed clock string. + Walks ``devices//log.json`` for every device directory, keeping the + highest-HLC copy of each record id -- records are id-stable, so the same + workout mirrored into two device logs dedups to one. + + Every device is walked because the phone's directory name is a per-install + uuid (since ffc6c72), not a fixed role id: it changes on every reinstall, + and old directories linger. Reading any single hardcoded device path is + guaranteed to go stale, which is exactly how phone sessions stopped + reaching this machine. + + Best-effort per device: an unreachable or corrupt device log yields fewer + records rather than raising, and says so at ``warning`` -- naming the + device and what is being dropped -- so a partial pull is never silent. + + ``strict`` controls only the *top-level* listing failure, where nothing at + all could be read. Callers that distinguish "nothing to sync" from "the + sync broke" pass ``strict=True`` to get the exception instead of an empty + dict, since an empty dict would otherwise report a total failure as the + benign "no records" case. """ - raw = json.loads(log_json) - if not isinstance(raw, dict): - msg = f"top-level sync payload is not a JSON object: {raw!r}" - raise TypeError(msg) + try: + devices = with_sync_retry( + lambda: client.list_directory(_DEVICES_PREFIX), + description="list synced device logs", + ) + except RemoteSyncError as exc: + _logger.warning( + "Could not list device logs under %s in %s/%s: %s — pulling NO " + "synced %s, so phone-logged workouts will not count", + _DEVICES_PREFIX, + SYNC_REPO_OWNER, + SYNC_REPO_NAME, + exc, + what, + ) + if strict: + raise + return {} - records = [Record.from_dict(data) for data in raw.values()] - candidates = [ - record - for record in records - if _PAYLOAD_FIELD in record.fields - and not _is_manual_payload(record.fields[_PAYLOAD_FIELD][0]) - ] - if not candidates: - return None - latest = max(candidates, key=lambda record: record.fields[_PAYLOAD_FIELD][1]) + merged: dict[str, tuple[dict, Hlc]] = {} + for device in devices: + path = f"{_DEVICES_PREFIX}/{device}/log.json" + try: + text = client.get_file_text(path) + except RemoteSyncError as exc: + _logger.warning( + "Could not fetch device log %s: %s — SKIPPING that device, so " + "any %s it holds will not count", + path, + exc, + what, + ) + continue + if text is None: + continue + try: + records = extract(text) + except (ValueError, KeyError, TypeError) as exc: + _logger.warning("Corrupt sync data at %s: %s", path, exc) + continue + for rid, (payload, hlc) in records.items(): + existing = merged.get(rid) + if existing is None or existing[1] < hlc: + merged[rid] = (payload, hlc) - payload = latest.fields[_PAYLOAD_FIELD][0] - if not isinstance(payload, dict): - msg = f"payload field is not a JSON object: {payload!r}" - raise TypeError(msg) - return payload + return merged def remote_client(github: RemoteStore) -> RemoteStore: @@ -121,70 +182,112 @@ def remote_client(github: RemoteStore) -> RemoteStore: return github -def pull_synced_workout() -> tuple[dict | None, str | None]: - """Return ``(data, error)``: the phone's last-synced workout, if any. +def sync_client() -> RemoteStore | None: + """Return the configured read client, or None if sync is set up nowhere. - - No token configured: ``(None, None)`` -- benign, sync is optional. - - Nothing pushed yet (repo reachable, path unused): ``(None, None)``. - - A real sync error (network, bad token, repo unreachable): - ``(None, )``. - - Corrupt/malformed sync data: ``(None, "corrupt sync data: ...")``. - - Success: ``(payload_dict, None)``. + A GitHub token is no longer required: Firebase has been the primary backend + since eb4ff01, so a Firebase-only machine must still sync. Previously this + module returned early whenever the PAT was missing, reporting "sync is OFF" + on a machine whose sync was working perfectly -- a false negative that hid + a live backend behind a legacy credential check. + + GitHub is used alone when only the PAT exists, Firebase alone when only + ``~/.config/crdt-sync/`` exists, and the mirrored union when both do. + ``None`` means neither is configured, which stays a benign, expected state. """ token = read_sync_token() - if token is None: - _logger.warning( - "Cannot pull the synced StrongLifts session: no sync token at %s — " - "GitHub sync is OFF, so only ADB/HTTP can verify a phone workout. " - "Create a fine-grained PAT with contents:write on %s/%s and save it " - "there (chmod 600)", - SYNC_TOKEN_FILE, - SYNC_REPO_OWNER, - SYNC_REPO_NAME, - ) - return None, None - - client = remote_client( + github = ( GitHubSyncClient( SYNC_REPO_OWNER, SYNC_REPO_NAME, token, timeout_seconds=SYNC_TIMEOUT_SECONDS, ) + if token is not None + else None ) - try: - text = with_sync_retry( - lambda: client.get_file_text(_LOG_PATH), - description="fetch the phone's workout log", - ) - except GitHubSyncError as exc: + if github is not None: + return remote_client(github) + if not CONFIG_FILE.is_file(): _logger.warning( - "Could not fetch the phone's workout log %s from %s/%s: %s — " - "falling back to ADB/HTTP verification", - _LOG_PATH, - SYNC_REPO_OWNER, - SYNC_REPO_NAME, + "Cannot pull synced workouts: no sync token at %s and no Firebase " + "config at %s — sync is OFF, so only ADB/HTTP can verify a phone " + "workout and phone-logged workouts will NOT count here", + SYNC_TOKEN_FILE, + CONFIG_FILE, + ) + return None + try: + return firebase_client_for("screen_locker") + except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc: + _logger.warning( + "Firebase is configured at %s but unusable, and there is no GitHub " + "token at %s to fall back to: %s — pulling NO synced workouts", + CONFIG_FILE, + SYNC_TOKEN_FILE, + exc, + ) + return None + + +def pull_synced_workout() -> tuple[dict | None, str | None]: + """Return ``(data, error)``: the last-synced StrongLifts session, if any. + + Merges sessions across EVERY device directory and returns the highest-HLC + one. It used to read a single hardcoded ``devices/phone/log.json``, which + silently went stale the moment the phone moved to a per-install uuid + (ffc6c72): this machine kept happily returning a session from 2026-07-27 + while newer ones sat unread one directory over. + + - Nothing configured anywhere: ``(None, None)`` -- benign, sync is optional. + - Nothing pushed yet: ``(None, None)``. + - A real sync error (network, bad token, backend unreachable): + ``(None, )``. + - Success: ``(payload_dict, None)``. + """ + client = sync_client() + if client is None: + return None, None + + try: + merged = _merge_device_records( + client, _session_records, "StrongLifts sessions", strict=True + ) + except RemoteSyncError as exc: + _logger.warning( + "Could not fetch synced workout logs under %s: %s — falling back " + "to ADB/HTTP verification", + _DEVICES_PREFIX, exc, ) return None, str(exc) - if text is None: + if not merged: return None, None - try: - payload = _latest_payload(text) - except (ValueError, KeyError, TypeError) as exc: - _logger.warning("Corrupt sync data at %s: %s", _LOG_PATH, exc) - return None, f"corrupt sync data: {exc}" - + _rid, (payload, _hlc) = max(merged.items(), key=lambda item: item[1][1]) return payload, None -def _manual_records(log_json: str) -> dict[str, tuple[dict, Hlc]]: - """Return ``{record_id: (payload, hlc)}`` for manual records in a log blob. +def _session_records(log_json: str) -> dict[str, tuple[dict, Hlc]]: + """Return ``{record_id: (payload, hlc)}`` for session records in a log blob. - Raises the same decode errors as :func:`_latest_payload`; callers treat a - corrupt device log as "no manual records from that device". + Raises the same decode errors as :func:`_manual_records`; callers treat a + corrupt device log as "no sessions from that device". + """ + return _records_matching(log_json, _is_session_payload) + + +def _records_matching( + log_json: str, predicate: Callable[[object], bool] +) -> dict[str, tuple[dict, Hlc]]: + """Return ``{record_id: (payload, hlc)}`` for payloads passing ``predicate``. + + Raises: + TypeError: If the top-level JSON isn't an object or a record's shape + doesn't match what :meth:`Record.from_dict` expects. + KeyError: If a record is missing an expected key. + ValueError: Via ``json.loads`` or :meth:`crdt_sync.Hlc.from_str`. """ raw = json.loads(log_json) if not isinstance(raw, dict): @@ -197,11 +300,20 @@ def _manual_records(log_json: str) -> dict[str, tuple[dict, Hlc]]: if field is None: continue payload, hlc = field - if _is_manual_payload(payload): + if predicate(payload): result[record.id] = (payload, hlc) return result +def _manual_records(log_json: str) -> dict[str, tuple[dict, Hlc]]: + """Return ``{record_id: (payload, hlc)}`` for manual records in a log blob. + + Raises the same decode errors as :func:`_records_matching`; callers treat a + corrupt device log as "no manual records from that device". + """ + return _records_matching(log_json, _is_manual_payload) + + def pull_all_manual_records() -> list[tuple[str, dict]]: """Return all synced manual-workout records across every device log. @@ -212,66 +324,9 @@ def pull_all_manual_records() -> list[tuple[str, dict]]: log yields fewer/no records rather than raising — manual sync, like session sync, is optional. """ - token = read_sync_token() - if token is None: - _logger.warning( - "Pulling NO synced manual workouts: no sync token at %s — GitHub " - "sync is OFF, so workouts logged on the phone will NOT count here. " - "Create a fine-grained PAT with contents:write on %s/%s and save it " - "there (chmod 600)", - SYNC_TOKEN_FILE, - SYNC_REPO_OWNER, - SYNC_REPO_NAME, - ) + client = sync_client() + if client is None: return [] - client = remote_client( - GitHubSyncClient( - SYNC_REPO_OWNER, - SYNC_REPO_NAME, - token, - timeout_seconds=SYNC_TIMEOUT_SECONDS, - ) - ) - try: - devices = with_sync_retry( - lambda: client.list_directory(_DEVICES_PREFIX), - description="list synced device logs", - ) - except GitHubSyncError as exc: - _logger.warning( - "Could not list device logs under %s in %s/%s: %s — pulling NO " - "synced manual workouts, so phone-logged workouts will not count", - _DEVICES_PREFIX, - SYNC_REPO_OWNER, - SYNC_REPO_NAME, - exc, - ) - return [] - - merged: dict[str, tuple[dict, Hlc]] = {} - for device in devices: - path = f"{_DEVICES_PREFIX}/{device}/log.json" - try: - text = client.get_file_text(path) - except GitHubSyncError as exc: - _logger.warning( - "Could not fetch device log %s: %s — SKIPPING that device, so " - "any manual workouts it holds will not count", - path, - exc, - ) - continue - if text is None: - continue - try: - records = _manual_records(text) - except (ValueError, KeyError, TypeError) as exc: - _logger.warning("Corrupt sync data at %s: %s", path, exc) - continue - for rid, (payload, hlc) in records.items(): - existing = merged.get(rid) - if existing is None or existing[1] < hlc: - merged[rid] = (payload, hlc) - + merged = _merge_device_records(client, _manual_records, "manual workouts") return [(rid, payload) for rid, (payload, _hlc) in merged.items()] diff --git a/screen_locker/screen_lock.py b/screen_locker/screen_lock.py index 7d0b7f4..0dab4ae 100755 --- a/screen_locker/screen_lock.py +++ b/screen_locker/screen_lock.py @@ -227,6 +227,16 @@ class ScreenLocker( # shutdown-time-only reward (see _apply_weekly_shutdown_bonus). self._check_heat_skip_exit() + def sync_now(self) -> None: + """Run one sync pass: publish this PC's workouts, ingest everyone's. + + The public entry point behind ``--sync-only``, which the + ``workout-sync.timer`` unit calls. Sync used to happen only inside the + locker's startup path, so a workout finished after login stayed + invisible until the next login. + """ + self._ingest_synced_manual_workouts() + def _ingest_synced_manual_workouts(self) -> None: """Sync manual workouts: publish this PC's, ingest everyone else's. @@ -342,6 +352,24 @@ if __name__ == "__main__": _sl.workout_data = {} run_status(_sl) + if "--sync-only" in sys.argv: + # Headless sync for the timer unit: pull other devices' workouts and + # apply their credit, with no Tk and no lock screen. Sync used to run + # ONLY at process start, so a workout finished after login was not seen + # until the next login — the timer closes that window. + # + # Same __init__ bypass as --status: this path touches only log_file and + # workout_data, and constructing the real ScreenLocker would build a UI. + logging.basicConfig( + level=logging.INFO, + format="%(levelname)s %(name)s: %(message)s", + ) + _sl = object.__new__(ScreenLocker) + _sl.log_file = Path(__file__).resolve().parent / "workout_log.json" + _sl.workout_data = {} + _sl.sync_now() + sys.exit(0) + demo_mode = True verify_only = "--verify-workout" in sys.argv diff --git a/screen_locker/tests/_workout_sync_fixtures.py b/screen_locker/tests/_workout_sync_fixtures.py new file mode 100644 index 0000000..06ea19f --- /dev/null +++ b/screen_locker/tests/_workout_sync_fixtures.py @@ -0,0 +1,97 @@ +"""Record/payload builders shared by the workout-sync test modules. + +Split out of ``test_workout_sync`` (and imported by it and its ``_part2``) +so neither file exceeds the repo's 400-line limit. Everything here builds +crdt-sync wire data the way the phone app actually emits it -- in +particular ``_session_payload`` carries NO ``kind`` field, because session +detection is by shape, and a fixture that invented a ``kind`` would hide +the very bug these tests exist to pin down. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING +from unittest.mock import MagicMock + +from crdt_sync import Hlc, Record + +if TYPE_CHECKING: + from pathlib import Path + from types import ModuleType + + import pytest + + +def _record_json(payload: dict, *, wall_time_ms: int = 1000) -> dict: + hlc = Hlc(wall_time_ms=wall_time_ms, counter=0, node_id="phone") + record = Record(id="x", fields={"payload": (payload, hlc)}) + return record.to_dict() + + +def _session_payload(**extra: object) -> dict: + """A StrongLifts session as the phone app's ``toJson()`` actually emits it. + + Deliberately carries NO ``kind`` field: session detection is by shape (an + ``exercises`` list), and keying it on ``kind`` is the bug that made real + sessions invisible. + """ + payload: dict = { + "exercises": [{"name": "Squat", "sets": 5}], + "duration_seconds": 1800, + } + payload.update(extra) + return payload + + +def _session_record_dict( + record_id: str, payload: dict, *, wall_time_ms: int = 1000 +) -> dict: + hlc = Hlc(wall_time_ms=wall_time_ms, counter=0, node_id="phone") + return Record(id=record_id, fields={"payload": (payload, hlc)}).to_dict() + + +def _manual_payload(**extra: object) -> dict: + payload = {"kind": "manual_workout", "date": "2026-07-13"} + payload.update(extra) + return payload + + +def _manual_record_dict( + record_id: str, payload: dict, *, wall_time_ms: int = 1000 +) -> dict: + hlc = Hlc(wall_time_ms=wall_time_ms, counter=0, node_id="phone") + return Record(id=record_id, fields={"payload": (payload, hlc)}).to_dict() + + +def _multi_device_client(device_logs: dict[str, object]) -> MagicMock: + """A client whose ``devices//log.json`` tree is ``device_logs``. + + An ``Exception`` value is raised on fetch, so a single dict describes + healthy, missing and unreachable devices at once. + """ + client = MagicMock() + client.list_directory.return_value = list(device_logs) + + def _get(path: str) -> object: + device = path.split("/")[-2] + value = device_logs[device] + if isinstance(value, Exception): + raise value + return value + + client.get_file_text.side_effect = _get + return client + + +def _firebase_config( + module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> Path: + """Point CONFIG_FILE at a real file so the Firebase branch is taken. + + The autouse ``_no_real_firebase_config`` fixture aims it at a nonexistent + path, so any test wanting the Firebase side must opt back in explicitly. + """ + config = tmp_path / "firebase.json" + config.write_text("{}", encoding="utf-8") + monkeypatch.setattr(module, "CONFIG_FILE", config) + return config diff --git a/screen_locker/tests/test_screen_lock_coverage_part1.py b/screen_locker/tests/test_screen_lock_coverage_part1.py index dc2f8d7..afb6126 100644 --- a/screen_locker/tests/test_screen_lock_coverage_part1.py +++ b/screen_locker/tests/test_screen_lock_coverage_part1.py @@ -342,3 +342,31 @@ class TestTryAutoUpgradeEarlyBirdRunnerUp: MagicMock(return_value=("not_verified", "no run")), ) assert locker._try_auto_upgrade_early_bird() is False + + +class TestSyncNow: + """Tests for sync_now, the public entry point behind ``--sync-only``.""" + + def test_runs_one_full_sync_pass( + self, mock_tk: MagicMock, mock_sys_exit: MagicMock, tmp_path: Path + ) -> None: + """The timer unit's entry point must actually push and ingest. + + ``workout-sync.timer`` calls this headlessly; if it stopped delegating, + the timer would run and silently sync nothing. + """ + locker = create_locker(mock_tk, tmp_path) + with ( + patch( + "screen_locker.screen_lock.pull_all_manual_records", + return_value=[("manual:x", {})], + ), + patch("screen_locker.screen_lock.push_pc_workouts") as push, + patch( + "screen_locker.screen_lock.ingest_manual_records", + return_value=["manual:x"], + ) as ingest, + ): + locker.sync_now() + push.assert_called_once() + ingest.assert_called_once() diff --git a/screen_locker/tests/test_workout_sync.py b/screen_locker/tests/test_workout_sync.py index 518b476..fda12ff 100644 --- a/screen_locker/tests/test_workout_sync.py +++ b/screen_locker/tests/test_workout_sync.py @@ -1,25 +1,35 @@ -"""Tests for the phone-workout GitHub sync pull (crdt-sync transport).""" +"""Tests for the phone-workout sync pull (crdt-sync transport). + +Covers the token read, the payload predicates, the per-log record extractors +and the cross-device merge. The backend selection (``sync_client`` / +``remote_client``) and the two public pulls live in ``_part2``. +""" from __future__ import annotations import json -from pathlib import Path -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock -from crdt_sync import ConfigError, GitHubSyncError, Hlc, Record +from crdt_sync import GitHubSyncError, Hlc, Record import pytest from screen_locker import _workout_sync +from screen_locker.tests._workout_sync_fixtures import ( + _manual_payload, + _manual_record_dict, + _multi_device_client, + _record_json, + _session_payload, + _session_record_dict, +) -# The autouse `_isolate_sync_token` fixture in conftest.py already redirects -# `_workout_sync.SYNC_TOKEN_FILE` into a per-test tmp_path, so tests here can -# write to it directly without needing their own isolation fixture. - - -def _record_json(payload: dict, *, wall_time_ms: int = 1000) -> dict: - hlc = Hlc(wall_time_ms=wall_time_ms, counter=0, node_id="phone") - record = Record(id="x", fields={"payload": (payload, hlc)}) - return record.to_dict() +# The autouse `isolate_sync_token` fixture (registered repo-wide via the +# `pytest_plugins` entry for `screen_locker.tests.test_sync_fixtures` in +# pyproject.toml) already redirects `_workout_sync.SYNC_TOKEN_FILE` into a +# per-test tmp_path, so tests here can write to it directly without needing +# their own isolation fixture. The autouse `_no_real_firebase_config` fixture +# in conftest.py likewise points `CONFIG_FILE` at a nonexistent path, so tests +# that want the Firebase branch must monkeypatch it back at a file they own. class TestReadSyncToken: @@ -35,116 +45,29 @@ class TestReadSyncToken: assert _workout_sync.read_sync_token() == "abc123" -class TestLatestPayload: - def test_returns_none_for_an_empty_log(self) -> None: - assert _workout_sync._latest_payload("{}") is None +class TestIsSessionPayload: + """Sessions are identified by SHAPE (an ``exercises`` list), not ``kind``.""" - def test_returns_the_only_records_payload(self) -> None: - log = {"a": _record_json({"succeeded": True})} - assert _workout_sync._latest_payload(json.dumps(log)) == {"succeeded": True} + def test_true_for_a_dict_with_an_exercises_list(self) -> None: + assert _workout_sync._is_session_payload(_session_payload()) is True - def test_returns_the_payload_with_the_greatest_clock(self) -> None: - log = { - "old": _record_json({"succeeded": False}, wall_time_ms=100), - "new": _record_json({"succeeded": True}, wall_time_ms=200), - } - assert _workout_sync._latest_payload(json.dumps(log)) == {"succeeded": True} + def test_true_even_with_an_empty_exercises_list(self) -> None: + assert _workout_sync._is_session_payload({"exercises": []}) is True - def test_skips_records_with_no_payload_field(self) -> None: - no_payload = Record( - id="a", - fields={"other": (1, Hlc(wall_time_ms=1, counter=0, node_id="phone"))}, - ) - log = {"a": no_payload.to_dict()} - assert _workout_sync._latest_payload(json.dumps(log)) is None + def test_false_for_a_manual_workout(self) -> None: + assert _workout_sync._is_session_payload(_manual_payload()) is False - def test_raises_type_error_when_top_level_is_not_an_object(self) -> None: - with pytest.raises(TypeError, match="not a JSON object"): - _workout_sync._latest_payload(json.dumps([1, 2, 3])) + def test_false_for_verified_records_sharing_the_same_log(self) -> None: + """``*_verified`` records are non-manual but have no ``exercises``.""" + assert _workout_sync._is_session_payload({"kind": "runnerup_verified"}) is False + assert _workout_sync._is_session_payload({"kind": "phone_verified"}) is False - def test_raises_type_error_when_payload_field_is_not_an_object(self) -> None: - log = {"a": _record_json("not-a-dict")} - with pytest.raises(TypeError, match="payload field is not a JSON object"): - _workout_sync._latest_payload(json.dumps(log)) + def test_false_when_exercises_is_not_a_list(self) -> None: + assert _workout_sync._is_session_payload({"exercises": "Squat"}) is False - def test_raises_key_error_for_a_malformed_record(self) -> None: - log = {"a": {"id": "a"}} # missing "fields" - with pytest.raises(KeyError): - _workout_sync._latest_payload(json.dumps(log)) - - def test_raises_value_error_for_invalid_json(self) -> None: - with pytest.raises(ValueError, match="Expecting property name"): - _workout_sync._latest_payload("{not valid json") - - -def _mock_client(*, get_file_text: object) -> MagicMock: - client = MagicMock() - if isinstance(get_file_text, Exception): - client.get_file_text.side_effect = get_file_text - else: - client.get_file_text.return_value = get_file_text - return client - - -class TestPullSyncedWorkout: - def test_returns_none_none_when_no_token_is_configured(self) -> None: - with patch.object(_workout_sync, "GitHubSyncClient") as client_cls: - assert _workout_sync.pull_synced_workout() == (None, None) - client_cls.assert_not_called() - - def test_returns_the_error_message_on_a_sync_error(self) -> None: - _workout_sync.SYNC_TOKEN_FILE.write_text("tok") - client = _mock_client(get_file_text=GitHubSyncError("offline")) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): - assert _workout_sync.pull_synced_workout() == (None, "offline") - - def test_returns_none_none_when_nothing_has_been_pushed_yet(self) -> None: - _workout_sync.SYNC_TOKEN_FILE.write_text("tok") - client = _mock_client(get_file_text=None) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): - assert _workout_sync.pull_synced_workout() == (None, None) - - def test_returns_the_payload_on_success(self) -> None: - _workout_sync.SYNC_TOKEN_FILE.write_text("tok") - log = json.dumps({"a": _record_json({"succeeded": True})}) - client = _mock_client(get_file_text=log) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): - assert _workout_sync.pull_synced_workout() == ({"succeeded": True}, None) - - def test_returns_a_corrupt_data_message_on_malformed_sync_data(self) -> None: - _workout_sync.SYNC_TOKEN_FILE.write_text("tok") - client = _mock_client(get_file_text="{not valid json") - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): - data, error = _workout_sync.pull_synced_workout() - assert data is None - assert error is not None - assert "corrupt sync data" in error - - -def _manual_payload(**extra: object) -> dict: - payload = {"kind": "manual_workout", "date": "2026-07-13"} - payload.update(extra) - return payload - - -def _manual_record_dict( - record_id: str, payload: dict, *, wall_time_ms: int = 1000 -) -> dict: - hlc = Hlc(wall_time_ms=wall_time_ms, counter=0, node_id="phone") - return Record(id=record_id, fields={"payload": (payload, hlc)}).to_dict() - - -class TestLatestPayloadSkipsManual: - def test_manual_only_log_yields_no_session(self) -> None: - log = {"m": _manual_record_dict("manual:1", _manual_payload())} - assert _workout_sync._latest_payload(json.dumps(log)) is None - - def test_returns_session_even_when_a_later_manual_exists(self) -> None: - log = { - "s": _record_json({"succeeded": True}, wall_time_ms=100), - "m": _manual_record_dict("manual:1", _manual_payload(), wall_time_ms=999), - } - assert _workout_sync._latest_payload(json.dumps(log)) == {"succeeded": True} + def test_false_for_a_non_dict_payload(self) -> None: + assert _workout_sync._is_session_payload("not-a-dict") is False + assert _workout_sync._is_session_payload(None) is False class TestIsManualPayload: @@ -152,14 +75,46 @@ class TestIsManualPayload: assert _workout_sync._is_manual_payload({"kind": "manual_workout"}) is True def test_false_for_session_or_non_dict(self) -> None: - assert _workout_sync._is_manual_payload({"succeeded": True}) is False + assert _workout_sync._is_manual_payload(_session_payload()) is False assert _workout_sync._is_manual_payload("not-a-dict") is False +class TestSessionRecords: + def test_returns_only_session_records(self) -> None: + log = { + "s": _session_record_dict("session:1", _session_payload()), + "m": _manual_record_dict("manual:1", _manual_payload()), + "v": _manual_record_dict("verified:1", {"kind": "runnerup_verified"}), + } + result = _workout_sync._session_records(json.dumps(log)) + assert list(result) == ["session:1"] + + def test_skips_records_without_a_payload_field(self) -> None: + no_payload = Record( + id="a", + fields={"other": (1, Hlc(wall_time_ms=1, counter=0, node_id="phone"))}, + ) + log = {"a": no_payload.to_dict()} + assert _workout_sync._session_records(json.dumps(log)) == {} + + def test_raises_type_error_when_top_level_is_not_an_object(self) -> None: + with pytest.raises(TypeError, match="not a JSON object"): + _workout_sync._session_records(json.dumps([1, 2, 3])) + + def test_raises_value_error_for_invalid_json(self) -> None: + with pytest.raises(ValueError, match="Expecting property name"): + _workout_sync._session_records("{not valid json") + + def test_raises_key_error_for_a_malformed_record(self) -> None: + log = {"a": {"id": "a"}} # missing "fields" + with pytest.raises(KeyError): + _workout_sync._session_records(json.dumps(log)) + + class TestManualRecords: def test_returns_only_manual_records(self) -> None: log = { - "s": _record_json({"succeeded": True}), + "s": _record_json(_session_payload()), "m": _manual_record_dict("manual:1", _manual_payload()), } result = _workout_sync._manual_records(json.dumps(log)) @@ -178,166 +133,146 @@ class TestManualRecords: _workout_sync._manual_records(json.dumps([1, 2, 3])) -def _multi_device_client(device_logs: dict[str, object]) -> MagicMock: - client = MagicMock() - client.list_directory.return_value = list(device_logs) +class TestMergeDeviceRecords: + """The walk over EVERY ``devices//log.json``, not one hardcoded path.""" - def _get(path: str) -> object: - device = path.split("/")[-2] - value = device_logs[device] - if isinstance(value, Exception): - raise value - return value - - client.get_file_text.side_effect = _get - return client - - -class TestPullAllManualRecords: - def test_returns_empty_when_no_token(self) -> None: - assert _workout_sync.pull_all_manual_records() == [] - - def test_returns_empty_when_listing_fails(self) -> None: - _workout_sync.SYNC_TOKEN_FILE.write_text("tok") - client = MagicMock() - client.list_directory.side_effect = GitHubSyncError("offline") - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): - assert _workout_sync.pull_all_manual_records() == [] - - def test_merges_manual_records_across_devices(self) -> None: - _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + def test_merges_records_across_every_device(self) -> None: client = _multi_device_client( { "phone": json.dumps( - {"a": _manual_record_dict("manual:a", _manual_payload())} + {"a": _session_record_dict("session:a", _session_payload())} ), "pc": json.dumps( - {"b": _manual_record_dict("manual:b", _manual_payload())} + {"b": _session_record_dict("session:b", _session_payload())} ), } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): - result = _workout_sync.pull_all_manual_records() - assert sorted(rid for rid, _ in result) == ["manual:a", "manual:b"] + merged = _workout_sync._merge_device_records( + client, _workout_sync._session_records, "sessions" + ) + assert sorted(merged) == ["session:a", "session:b"] - def test_skips_missing_and_corrupt_device_logs(self) -> None: - _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + def test_dedups_the_same_id_keeping_the_highest_clock(self) -> None: client = _multi_device_client( { "phone": json.dumps( - {"a": _manual_record_dict("manual:a", _manual_payload())} + { + "a": _session_record_dict( + "session:a", + _session_payload(note="OLD"), + wall_time_ms=100, + ) + } + ), + "pc": json.dumps( + { + "a": _session_record_dict( + "session:a", + _session_payload(note="NEW"), + wall_time_ms=200, + ) + } + ), + } + ) + merged = _workout_sync._merge_device_records( + client, _workout_sync._session_records, "sessions" + ) + assert len(merged) == 1 + assert merged["session:a"][0]["note"] == "NEW" + + def test_ignores_a_lower_clock_duplicate_seen_later(self) -> None: + client = _multi_device_client( + { + "phone": json.dumps( + { + "a": _session_record_dict( + "session:a", + _session_payload(note="NEW"), + wall_time_ms=200, + ) + } + ), + "pc": json.dumps( + { + "a": _session_record_dict( + "session:a", + _session_payload(note="OLD"), + wall_time_ms=100, + ) + } + ), + } + ) + merged = _workout_sync._merge_device_records( + client, _workout_sync._session_records, "sessions" + ) + assert merged["session:a"][0]["note"] == "NEW" + + def test_skips_a_corrupt_device_log(self) -> None: + client = _multi_device_client( + { + "good": json.dumps( + {"a": _session_record_dict("session:a", _session_payload())} ), - "gone": GitHubSyncError("404"), - "empty": None, "corrupt": "{not json", } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): - result = _workout_sync.pull_all_manual_records() - assert [rid for rid, _ in result] == ["manual:a"] + merged = _workout_sync._merge_device_records( + client, _workout_sync._session_records, "sessions" + ) + assert list(merged) == ["session:a"] - def test_dedups_same_id_keeping_highest_clock(self) -> None: - _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + def test_skips_an_unreachable_device(self) -> None: client = _multi_device_client( { - "phone": json.dumps( - { - "a": _manual_record_dict( - "manual:a", - _manual_payload(cost="OLD"), - wall_time_ms=100, - ) - } - ), - "pc": json.dumps( - { - "a": _manual_record_dict( - "manual:a", - _manual_payload(cost="NEW"), - wall_time_ms=200, - ) - } + "good": json.dumps( + {"a": _session_record_dict("session:a", _session_payload())} ), + "gone": GitHubSyncError("404"), + "empty": None, } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): - result = _workout_sync.pull_all_manual_records() - assert len(result) == 1 - assert result[0][1]["cost"] == "NEW" - - def test_ignores_a_lower_clock_duplicate_seen_later(self) -> None: - _workout_sync.SYNC_TOKEN_FILE.write_text("tok") - client = _multi_device_client( - { - "phone": json.dumps( - { - "a": _manual_record_dict( - "manual:a", - _manual_payload(cost="NEW"), - wall_time_ms=200, - ) - } - ), - "pc": json.dumps( - { - "a": _manual_record_dict( - "manual:a", - _manual_payload(cost="OLD"), - wall_time_ms=100, - ) - } - ), - } + merged = _workout_sync._merge_device_records( + client, _workout_sync._session_records, "sessions" ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): - result = _workout_sync.pull_all_manual_records() - assert len(result) == 1 - assert result[0][1]["cost"] == "NEW" + assert list(merged) == ["session:a"] - -class TestRemoteClient: - """Which backend the workout pull reads from 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( - _workout_sync, "CONFIG_FILE", Path("/nonexistent/firebase.json") + def test_returns_empty_on_a_listing_failure_when_not_strict(self) -> None: + client = MagicMock() + client.list_directory.side_effect = GitHubSyncError("offline") + merged = _workout_sync._merge_device_records( + client, _workout_sync._session_records, "sessions" ) - github = object() + assert merged == {} - assert _workout_sync.remote_client(github) is github + def test_reraises_on_a_listing_failure_when_strict(self) -> None: + """Callers separating "nothing to sync" from "sync broke" need the raise.""" + client = MagicMock() + client.list_directory.side_effect = GitHubSyncError("offline") + with pytest.raises(GitHubSyncError, match="offline"): + _workout_sync._merge_device_records( + client, _workout_sync._session_records, "sessions", strict=True + ) - def test_mirrors_to_github_when_configured( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + def test_warns_about_a_listing_failure( + self, caplog: pytest.LogCaptureFixture ) -> None: - """Configured: reads union both, so either backend's workout counts.""" - config = tmp_path / "firebase.json" - config.write_text("{}", encoding="utf-8") - monkeypatch.setattr(_workout_sync, "CONFIG_FILE", config) - monkeypatch.setattr( - _workout_sync, - "mirror_client_for", - lambda _app, client: ("mirror", client), - ) - github = object() + """A total pull failure must never be silent.""" + client = MagicMock() + client.list_directory.side_effect = GitHubSyncError("offline") + with caplog.at_level("WARNING"): + _workout_sync._merge_device_records( + client, _workout_sync._session_records, "sessions" + ) + assert "offline" in caplog.text - assert _workout_sync.remote_client(github) == ("mirror", github) - - def test_falls_back_when_firebase_is_unusable( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + def test_warns_about_a_skipped_device( + self, caplog: pytest.LogCaptureFixture ) -> None: - """A broken Firebase must degrade to GitHub, never fail the pull.""" - config = tmp_path / "firebase.json" - config.write_text("{}", encoding="utf-8") - monkeypatch.setattr(_workout_sync, "CONFIG_FILE", config) - - def _boom(*_args: object, **_kwargs: object) -> None: - message = "no password" - raise ConfigError(message) - - monkeypatch.setattr(_workout_sync, "mirror_client_for", _boom) - github = object() - - assert _workout_sync.remote_client(github) is github + client = _multi_device_client({"gone": GitHubSyncError("404")}) + with caplog.at_level("WARNING"): + _workout_sync._merge_device_records( + client, _workout_sync._session_records, "sessions" + ) + assert "SKIPPING that device" in caplog.text diff --git a/screen_locker/tests/test_workout_sync_part2.py b/screen_locker/tests/test_workout_sync_part2.py new file mode 100644 index 0000000..c2a0b77 --- /dev/null +++ b/screen_locker/tests/test_workout_sync_part2.py @@ -0,0 +1,400 @@ +"""Backend selection and the two public pulls, split from ``test_workout_sync``. + +``sync_client`` decides WHICH backend is read; ``pull_synced_workout`` and +``pull_all_manual_records`` are what the locker actually calls. +""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING +from unittest.mock import MagicMock, patch + +from crdt_sync import ( + ConfigError, + FirebaseAuthError, + FirebaseSyncError, + GitHubSyncError, +) +import pytest + +from screen_locker import _workout_sync +from screen_locker.tests._workout_sync_fixtures import ( + _firebase_config, + _manual_payload, + _manual_record_dict, + _multi_device_client, + _session_payload, + _session_record_dict, +) + +if TYPE_CHECKING: + from pathlib import Path + + +class TestSyncClient: + """Every configuration of (GitHub token, Firebase config), plus failure.""" + + def test_returns_none_when_nothing_is_configured(self) -> None: + assert _workout_sync.sync_client() is None + + def test_warns_when_nothing_is_configured( + self, caplog: pytest.LogCaptureFixture + ) -> None: + with caplog.at_level("WARNING"): + _workout_sync.sync_client() + assert "sync is OFF" in caplog.text + + def test_uses_github_alone_when_only_the_token_exists(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + github = MagicMock() + with patch.object(_workout_sync, "GitHubSyncClient", return_value=github): + assert _workout_sync.sync_client() is github + + def test_mirrors_when_both_the_token_and_firebase_exist( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + _firebase_config(_workout_sync, tmp_path, monkeypatch) + github = MagicMock() + monkeypatch.setattr( + _workout_sync, "mirror_client_for", lambda _app, client: ("mirror", client) + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=github): + assert _workout_sync.sync_client() == ("mirror", github) + + def test_uses_firebase_alone_when_only_the_config_exists( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A Firebase-only machine must still sync -- no PAT required.""" + _firebase_config(_workout_sync, tmp_path, monkeypatch) + firebase = MagicMock() + monkeypatch.setattr(_workout_sync, "firebase_client_for", lambda _app: firebase) + assert _workout_sync.sync_client() is firebase + + @pytest.mark.parametrize( + "error", + [ + ConfigError("no password"), + FirebaseAuthError("bad credentials"), + FirebaseSyncError("backend down"), + ], + ) + def test_returns_none_when_firebase_only_is_unusable( + self, error: Exception, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """No PAT to fall back to: report it loudly, do not raise.""" + _firebase_config(_workout_sync, tmp_path, monkeypatch) + + def _boom(*_args: object, **_kwargs: object) -> None: + raise error + + monkeypatch.setattr(_workout_sync, "firebase_client_for", _boom) + assert _workout_sync.sync_client() is None + + def test_warns_when_firebase_only_is_unusable( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, + ) -> None: + _firebase_config(_workout_sync, tmp_path, monkeypatch) + + def _boom(*_args: object, **_kwargs: object) -> None: + message = "backend down" + raise FirebaseSyncError(message) + + monkeypatch.setattr(_workout_sync, "firebase_client_for", _boom) + with caplog.at_level("WARNING"): + _workout_sync.sync_client() + assert "backend down" in caplog.text + + +class TestPullSyncedWorkout: + def test_returns_none_none_when_no_token_is_configured(self) -> None: + with patch.object(_workout_sync, "GitHubSyncClient") as client_cls: + assert _workout_sync.pull_synced_workout() == (None, None) + client_cls.assert_not_called() + + def test_returns_the_error_message_on_a_sync_error(self) -> None: + """A failed device LISTING is a real error, not "nothing to sync".""" + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = MagicMock() + client.list_directory.side_effect = GitHubSyncError("offline") + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + assert _workout_sync.pull_synced_workout() == (None, "offline") + + def test_does_not_propagate_a_firebase_sync_error(self) -> None: + """Regression: FirebaseSyncError is a SIBLING of GitHubSyncError. + + Catching only ``GitHubSyncError`` let a Firebase failure escape and + crash the locker with status=1/FAILURE. It must come back as an error + tuple like any other backend failure. + """ + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = MagicMock() + client.list_directory.side_effect = FirebaseSyncError("firebase exploded") + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + data, error = _workout_sync.pull_synced_workout() + assert data is None + assert error == "firebase exploded" + + def test_does_not_propagate_a_firebase_error_on_a_firebase_only_machine( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The crash's real shape: no PAT, Firebase primary, backend throws. + + The configuration the locker actually died in, pinned in its own right + so a future GitHub-only narrowing of the handler cannot pass. + """ + _firebase_config(_workout_sync, tmp_path, monkeypatch) + client = MagicMock() + client.list_directory.side_effect = FirebaseSyncError("firebase exploded") + monkeypatch.setattr(_workout_sync, "firebase_client_for", lambda _app: client) + data, error = _workout_sync.pull_synced_workout() + assert data is None + assert error == "firebase exploded" + + def test_returns_none_none_when_nothing_has_been_pushed_yet(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = _multi_device_client({"phone": None}) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + assert _workout_sync.pull_synced_workout() == (None, None) + + def test_returns_none_none_when_there_are_no_devices_at_all(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = _multi_device_client({}) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + assert _workout_sync.pull_synced_workout() == (None, None) + + def test_returns_the_payload_on_success(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + payload = _session_payload() + client = _multi_device_client( + {"phone": json.dumps({"a": _session_record_dict("session:a", payload)})} + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + assert _workout_sync.pull_synced_workout() == (payload, None) + + def test_finds_a_session_in_a_uuid_named_device_directory(self) -> None: + """The actual bug: the phone's dir is a per-install uuid, not "phone". + + Reading a single hardcoded ``devices/phone/log.json`` returned a stale + session forever while newer ones sat unread one directory over. + """ + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + fresh = _session_payload(note="FROM-UUID-DIR") + client = _multi_device_client( + { + "pc": json.dumps( + { + "old": _session_record_dict( + "session:old", + _session_payload(note="STALE"), + wall_time_ms=100, + ) + } + ), + "0f1d2c3b-4a59-4c6d-8e7f-a0b1c2d3e4f5": json.dumps( + { + "new": _session_record_dict( + "session:new", fresh, wall_time_ms=200 + ) + } + ), + } + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + assert _workout_sync.pull_synced_workout() == (fresh, None) + + def test_returns_the_highest_clock_session_across_devices(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + newest = _session_payload(note="NEWEST") + client = _multi_device_client( + { + "phone-a": json.dumps( + { + "a": _session_record_dict( + "session:a", + _session_payload(note="OLDER"), + wall_time_ms=100, + ) + } + ), + "phone-b": json.dumps( + {"b": _session_record_dict("session:b", newest, wall_time_ms=300)} + ), + "phone-c": json.dumps( + { + "c": _session_record_dict( + "session:c", + _session_payload(note="MIDDLE"), + wall_time_ms=200, + ) + } + ), + } + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + data, error = _workout_sync.pull_synced_workout() + assert error is None + assert data == newest + + def test_ignores_non_session_records_in_the_same_logs(self) -> None: + """``phone_verified`` / ``runnerup_verified`` share these device logs. + + They are non-manual but have no ``exercises``, so "not manual" alone + would hand back a record with none of the fields a caller expects -- + even though here they carry the HIGHEST clocks in the log. + """ + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + session = _session_payload(note="REAL-SESSION") + client = _multi_device_client( + { + "phone": json.dumps( + { + "s": _session_record_dict( + "session:a", session, wall_time_ms=100 + ), + "v": _session_record_dict( + "verified:1", + {"kind": "phone_verified", "date": "2026-08-10"}, + wall_time_ms=900, + ), + "r": _session_record_dict( + "verified:2", + {"kind": "runnerup_verified", "date": "2026-08-10"}, + wall_time_ms=950, + ), + "m": _manual_record_dict( + "manual:1", _manual_payload(), wall_time_ms=999 + ), + } + ) + } + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + data, error = _workout_sync.pull_synced_workout() + assert error is None + assert data == session + + def test_skips_a_corrupt_device_log_instead_of_failing(self) -> None: + """A corrupt log costs that device's sessions, not the whole pull.""" + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + payload = _session_payload() + client = _multi_device_client( + { + "corrupt": "{not valid json", + "phone": json.dumps({"a": _session_record_dict("session:a", payload)}), + } + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + assert _workout_sync.pull_synced_workout() == (payload, None) + + def test_returns_none_none_when_every_device_log_is_corrupt(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = _multi_device_client({"corrupt": "{not valid json"}) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + assert _workout_sync.pull_synced_workout() == (None, None) + + +class TestPullAllManualRecords: + def test_returns_empty_when_no_token(self) -> None: + assert _workout_sync.pull_all_manual_records() == [] + + def test_returns_empty_when_listing_fails(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = MagicMock() + client.list_directory.side_effect = GitHubSyncError("offline") + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + assert _workout_sync.pull_all_manual_records() == [] + + def test_merges_manual_records_across_devices(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = _multi_device_client( + { + "phone": json.dumps( + {"a": _manual_record_dict("manual:a", _manual_payload())} + ), + "pc": json.dumps( + {"b": _manual_record_dict("manual:b", _manual_payload())} + ), + } + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + result = _workout_sync.pull_all_manual_records() + assert sorted(rid for rid, _ in result) == ["manual:a", "manual:b"] + + def test_skips_missing_and_corrupt_device_logs(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = _multi_device_client( + { + "phone": json.dumps( + {"a": _manual_record_dict("manual:a", _manual_payload())} + ), + "gone": GitHubSyncError("404"), + "empty": None, + "corrupt": "{not json", + } + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + result = _workout_sync.pull_all_manual_records() + assert [rid for rid, _ in result] == ["manual:a"] + + def test_dedups_same_id_keeping_highest_clock(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = _multi_device_client( + { + "phone": json.dumps( + { + "a": _manual_record_dict( + "manual:a", + _manual_payload(cost="OLD"), + wall_time_ms=100, + ) + } + ), + "pc": json.dumps( + { + "a": _manual_record_dict( + "manual:a", + _manual_payload(cost="NEW"), + wall_time_ms=200, + ) + } + ), + } + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + result = _workout_sync.pull_all_manual_records() + assert len(result) == 1 + assert result[0][1]["cost"] == "NEW" + + def test_ignores_a_lower_clock_duplicate_seen_later(self) -> None: + _workout_sync.SYNC_TOKEN_FILE.write_text("tok") + client = _multi_device_client( + { + "phone": json.dumps( + { + "a": _manual_record_dict( + "manual:a", + _manual_payload(cost="NEW"), + wall_time_ms=200, + ) + } + ), + "pc": json.dumps( + { + "a": _manual_record_dict( + "manual:a", + _manual_payload(cost="OLD"), + wall_time_ms=100, + ) + } + ), + } + ) + with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + result = _workout_sync.pull_all_manual_records() + assert len(result) == 1 + assert result[0][1]["cost"] == "NEW" diff --git a/screen_locker/tests/test_workout_sync_part3.py b/screen_locker/tests/test_workout_sync_part3.py new file mode 100644 index 0000000..369352e --- /dev/null +++ b/screen_locker/tests/test_workout_sync_part3.py @@ -0,0 +1,62 @@ +"""``remote_client`` mirror selection, split from ``test_workout_sync_part2``. + +Kept separate from the pulls so no workout-sync test file exceeds the repo's +400-line limit. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import TYPE_CHECKING + +from crdt_sync import ConfigError + +from screen_locker import _workout_sync +from screen_locker.tests._workout_sync_fixtures import _firebase_config + +if TYPE_CHECKING: + import pytest + + +class TestRemoteClient: + """Which backend the workout pull reads from 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( + _workout_sync, "CONFIG_FILE", Path("/nonexistent/firebase.json") + ) + github = object() + + assert _workout_sync.remote_client(github) is github + + def test_mirrors_to_github_when_configured( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Configured: reads union both, so either backend's workout counts.""" + _firebase_config(_workout_sync, tmp_path, monkeypatch) + monkeypatch.setattr( + _workout_sync, + "mirror_client_for", + lambda _app, client: ("mirror", client), + ) + github = object() + + assert _workout_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 pull.""" + _firebase_config(_workout_sync, tmp_path, monkeypatch) + + def _boom(*_args: object, **_kwargs: object) -> None: + message = "no password" + raise ConfigError(message) + + monkeypatch.setattr(_workout_sync, "mirror_client_for", _boom) + github = object() + + assert _workout_sync.remote_client(github) is github diff --git a/stronglift_replacement/workout_app/lib/main.dart b/stronglift_replacement/workout_app/lib/main.dart index 9a4f799..59f38e1 100644 --- a/stronglift_replacement/workout_app/lib/main.dart +++ b/stronglift_replacement/workout_app/lib/main.dart @@ -17,7 +17,20 @@ 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(); - await BackupService.instance.requestStoragePermission(); + // 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(); + 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.', + ); + } await StorageService.init(); await StorageService.instance.restoreFromBackupIfNeeded(); await HttpServerService.instance.start(); diff --git a/stronglift_replacement/workout_app/lib/screens/history_screen.dart b/stronglift_replacement/workout_app/lib/screens/history_screen.dart index 23be895..afc9e06 100644 --- a/stronglift_replacement/workout_app/lib/screens/history_screen.dart +++ b/stronglift_replacement/workout_app/lib/screens/history_screen.dart @@ -57,6 +57,12 @@ class _HistoryScreenState extends State { } Future _load() async { + // Put any of this device's own sessions that only exist remotely back into + // the local table FIRST, so they show up in this same load. A reinstall + // wipes local history while Firebase/GitHub keep it; without this the app + // silently shows less history than actually happened. + final remote = await _readSyncedPayloads(); + await StorageService.instance.restoreSyncedSessions(remote); final rows = await StorageService.instance.getWorkoutHistory(limit: 200); final names = []; final seen = {}; @@ -76,7 +82,7 @@ class _HistoryScreenState extends State { state = await StorageService.instance.getExerciseState(_selected); // coverage:ignore-end } - final synced = await _loadSyncedWorkouts(); + final synced = _syncedOnly(remote); if (mounted) { setState(() { _rows = rows; @@ -88,20 +94,27 @@ class _HistoryScreenState extends State { } } + /// Every payload the sync backends hold, across all devices. + /// + /// Read once per load and used twice: to restore this device's own missing + /// sessions into local history, and to list the PC-only records below them. + Future>> _readSyncedPayloads() => + WorkoutSyncService( + httpClient: widget.httpClient, + ).readMergedWorkoutPayloads(); + /// Workouts the PC published that this phone has no local record of. /// /// The PC pushes its whole `workout_log.json` (RunnerUp runs and manual /// entries included), so pulling them here is what makes both devices show /// the SAME history. Sorted newest-first to match the local session list. - Future>> _loadSyncedWorkouts() async { - final payloads = await WorkoutSyncService( - httpClient: widget.httpClient, - ).readMergedWorkoutPayloads(); - final synced = - payloads.where((p) => _kSyncedOnlyKinds.contains(p['kind'])).toList() - ..sort((a, b) => '${b['date']}'.compareTo('${a['date']}')); - return synced; - } + /// StrongLifts sessions are deliberately excluded: they are restored into + /// local history instead, so listing them here too would double them up. + List> _syncedOnly( + List> payloads, + ) => + payloads.where((p) => _kSyncedOnlyKinds.contains(p['kind'])).toList() + ..sort((a, b) => '${b['date']}'.compareTo('${a['date']}')); Future _pickExercise(String name) async { ExerciseState? state; diff --git a/stronglift_replacement/workout_app/lib/screens/settings_screen.dart b/stronglift_replacement/workout_app/lib/screens/settings_screen.dart index 2c3461f..cedef24 100644 --- a/stronglift_replacement/workout_app/lib/screens/settings_screen.dart +++ b/stronglift_replacement/workout_app/lib/screens/settings_screen.dart @@ -59,10 +59,14 @@ class _SettingsScreenState extends State { final Map _successThresholds = {}; final Map _failThresholds = {}; final Map _weights = {}; + final Map _reps = {}; // Debounce weight saves to avoid resetting streaks on every tap. final Map _weightTimers = {}; + // Same debounce for reps, for the same reason. + final Map _repsTimers = {}; + final _tokenController = TextEditingController(); final _emailController = TextEditingController(); final _passwordController = TextEditingController(); @@ -154,6 +158,9 @@ class _SettingsScreenState extends State { for (final t in _weightTimers.values) { t.cancel(); } + for (final t in _repsTimers.values) { + t.cancel(); + } _tokenController.dispose(); _emailController.dispose(); _passwordController.dispose(); @@ -169,6 +176,7 @@ class _SettingsScreenState extends State { _successThresholds[s.name] = s.successThreshold; _failThresholds[s.name] = s.failThreshold; _weights[s.name] = s.weight; + _reps[s.name] = s.reps; } _tokenController.text = syncSettings.token; _loading = false; @@ -299,6 +307,14 @@ class _SettingsScreenState extends State { }); } + void _onRepsChanged(String name, int value) { + setState(() => _reps[name] = value); + _repsTimers[name]?.cancel(); + _repsTimers[name] = Timer(const Duration(milliseconds: 600), () { + unawaited(StorageService.instance.setExerciseReps(name, value)); + }); + } + Future _onThresholdChanged(String name, int success, int fail) async { setState(() { _successThresholds[name] = success; @@ -404,6 +420,28 @@ class _SettingsScreenState extends State { ); }), const SizedBox(height: 20), + const _SectionHeader('TARGET REPS'), + const SizedBox(height: 4), + Text( + 'Override target reps per set. Resets streak counters. ' + 'Progression only ever raises reps, so this is the only way ' + 'to lower one.', + style: TextStyle( + color: colorScheme.onSurfaceVariant, + fontSize: AppTextSize.caption, + ), + ), + const SizedBox(height: 12), + ..._orderedNames.map((name) { + final r = _reps[name]; + if (r == null) return const SizedBox.shrink(); + return _RepsRow( + name: name, + reps: r, + onChanged: (v) => _onRepsChanged(name, v), + ); + }), + const SizedBox(height: 20), const _SectionHeader('PROGRESSION THRESHOLDS'), const SizedBox(height: 4), Text( @@ -559,6 +597,59 @@ class _SectionHeader extends StatelessWidget { } } +class _RepsRow extends StatelessWidget { + const _RepsRow({ + required this.name, + required this.reps, + required this.onChanged, + }); + + final String name; + final int reps; + final ValueChanged onChanged; + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + return Padding( + padding: const EdgeInsets.only(bottom: 10), + child: Row( + children: [ + Expanded( + child: Text( + name, + style: TextStyle( + color: colorScheme.onSurfaceVariant, + fontSize: AppTextSize.label, + ), + ), + ), + _StepperButton( + icon: Icons.remove, + onTap: () => onChanged((reps - 1).clamp(1, 999)), + ), + SizedBox( + width: 72, + child: Text( + '$reps reps', + textAlign: TextAlign.center, + style: TextStyle( + color: colorScheme.onSurface, + fontSize: AppTextSize.label, + fontWeight: FontWeight.bold, + ), + ), + ), + _StepperButton( + icon: Icons.add, + onTap: () => onChanged((reps + 1).clamp(1, 999)), + ), + ], + ), + ); + } +} + class _WeightRow extends StatelessWidget { const _WeightRow({ required this.name, diff --git a/stronglift_replacement/workout_app/lib/services/backup_service.dart b/stronglift_replacement/workout_app/lib/services/backup_service.dart index 9ea749f..37ea3ee 100644 --- a/stronglift_replacement/workout_app/lib/services/backup_service.dart +++ b/stronglift_replacement/workout_app/lib/services/backup_service.dart @@ -14,7 +14,7 @@ library; import 'dart:convert'; import 'dart:io'; -import 'package:flutter/foundation.dart' show visibleForTesting; +import 'package:flutter/foundation.dart' show debugPrint, visibleForTesting; import 'package:permission_handler/permission_handler.dart'; /// Default directory backups live in on external storage. @@ -87,9 +87,13 @@ class BackupService { /// Writes [data] to the backup path as pretty-printed JSON. /// - /// Silently swallows errors (e.g. permission not granted, no external - /// storage) so the caller never crashes. - Future export(Map data) async { + /// Never throws — a failed backup must not crash a workout in progress — but + /// it is NOT silent: a backup that quietly stops working is invisible until + /// the reinstall that needed it, which is exactly how the 2026-08-05 wipe + /// destroyed months of progression. + /// + /// Returns true if the backup was written. + Future export(Map data) async { try { final dir = Directory(_baseDir); if (!dir.existsSync()) { @@ -98,8 +102,14 @@ class BackupService { await File( _backupPath, ).writeAsString(const JsonEncoder.withIndent(' ').convert(data)); - } on Exception { - // Backup is best-effort; never throw. + return true; + } on Exception catch (e) { + debugPrint( + 'WorkoutApp: BACKUP FAILED to $_backupPath: $e — progression state is ' + 'now unprotected against an uninstall/reinstall. Check that storage ' + 'permission is granted.', + ); + return false; } } @@ -107,13 +117,29 @@ class BackupService { /// Returns the parsed backup JSON, or null if the file does not exist or /// is unreadable. + /// + /// Distinguishes the two cases in the log: "no backup exists" is normal on a + /// genuinely first install, whereas "a backup exists but could not be read" + /// means a restore is about to fall through to defaults and silently lose + /// real data. The second must never pass unnoticed. Future?> readBackup() async { + final f = File(_backupPath); try { - final f = File(_backupPath); - if (!f.existsSync()) return null; + if (!f.existsSync()) { + debugPrint( + 'WorkoutApp: no backup at $_backupPath — nothing to restore. Normal ' + 'on a first install; on a REINSTALL it means the backup was lost.', + ); + return null; + } final raw = await f.readAsString(); return jsonDecode(raw) as Map; - } on Exception { + } on Exception catch (e) { + debugPrint( + 'WorkoutApp: BACKUP UNREADABLE at $_backupPath: $e — a backup EXISTS ' + 'but cannot be parsed, so restore will fall through to defaults and ' + 'lose progression state. Do not let the app overwrite it.', + ); return null; } } diff --git a/stronglift_replacement/workout_app/lib/services/storage_service.dart b/stronglift_replacement/workout_app/lib/services/storage_service.dart index 17a3da5..3959caa 100644 --- a/stronglift_replacement/workout_app/lib/services/storage_service.dart +++ b/stronglift_replacement/workout_app/lib/services/storage_service.dart @@ -316,6 +316,22 @@ class StorageService { ); } + /// Sets the target reps for [name], resetting streaks. + /// + /// Progression can only ever raise reps (by one, and only once an exercise + /// is at its max weight), so without this there is no way to lower a rep + /// target that a defaults re-seed set wrong -- the 2026-08-05 wipe dropped + /// Situp from 31 to the default 30 with no way back short of editing the DB. + Future setExerciseReps(String name, int reps) async { + await _db.update( + 'exercise_state', + {'reps': reps, 'success_streak': 0, 'fail_streak': 0}, + where: 'name = ?', + whereArgs: [name], + ); + unawaited(_backupNow()); + } + /// Sets the working weight for [name], resetting streaks. Future setExerciseWeight(String name, double weight) async { await _db.update( @@ -449,6 +465,76 @@ class StorageService { } /// Returns up to [limit] rows from workout history, newest first. + /// Inserts synced sessions this device has no local record of. + /// + /// These are the user's OWN completed workouts, already in Firebase/GitHub; + /// a reinstall wipes the local `workout_history` table while the remote copy + /// survives, so the app ends up showing far less history than actually + /// happened. This puts the real records back rather than inventing anything. + /// + /// Idempotent: a session is matched on (date, start_time) taken from the + /// payload, so repeated syncs never duplicate a row. Sessions the device + /// already has always win -- the local row is the one the progression + /// engine acted on. + /// + /// Returns the number of sessions restored. + Future restoreSyncedSessions( + List> payloads, + ) async { + if (payloads.isEmpty) return 0; + final existing = await _db.query('workout_history', columns: ['json']); + final seen = { + for (final row in existing) _sessionKey(_decodeJson(row['json'])), + }; + + var restored = 0; + for (final payload in payloads) { + // Only real sessions: a payload with no exercises is a PC-side + // runnerup/manual record, which belongs in the synced list, not here. + if (payload['exercises'] is! List) continue; + final key = _sessionKey(payload); + if (key.isEmpty || seen.contains(key)) continue; + seen.add(key); + await _db.insert('workout_history', { + 'date': payload['date'] as String? ?? '', + 'workout_type': payload['workout_type'] as String? ?? '', + 'duration_seconds': (payload['duration_seconds'] as num?)?.toInt() ?? 0, + 'succeeded': (payload['succeeded'] as bool? ?? false) ? 1 : 0, + 'json': jsonEncode(payload), + }); + restored++; + } + if (restored > 0) { + debugPrint('WorkoutApp: restored $restored synced session(s) locally.'); + unawaited(_backupNow()); + } + return restored; + } + + /// Identity of a session: its date plus start time, both from the payload. + /// + /// `start_time` alone would be enough, but pairing it with the date keeps + /// the key readable in logs and survives a payload missing either field. + static String _sessionKey(Map? payload) { + if (payload == null) return ''; + final start = payload['start_time'] as String? ?? ''; + final date = payload['date'] as String? ?? ''; + return start.isEmpty && date.isEmpty ? '' : '$date|$start'; + } + + static Map? _decodeJson(Object? raw) { + if (raw is! String || raw.isEmpty) return null; + try { + final decoded = jsonDecode(raw); + return decoded is Map ? decoded : null; + } on FormatException { + // A corrupt local row must not block restoring the rest; it simply + // cannot be matched against, so it is treated as "not seen". + return null; + } + } + + /// Returns up to [limit] past sessions, newest first. Future>> getWorkoutHistory({ int limit = 60, }) async { @@ -491,10 +577,34 @@ class StorageService { // ── Backup / restore ─────────────────────────────────────────────────────── /// Exports all persistent data to external storage as a JSON snapshot. + /// + /// Refuses to overwrite a richer existing backup with a freshly-seeded, + /// history-less DB. That combination is never a real state worth saving: it + /// only happens when a restore failed (permission denied, unreadable file) + /// and the DB is sitting at [workoutA]/[workoutB] defaults. The old code + /// exported anyway on the first write after such a seed, replacing the only + /// off-device copy of months of progression with default rows — the step + /// that turned the 2026-08-05 reinstall from recoverable into permanent. Future _backupNow() async { final exerciseRows = await _db.query('exercise_state'); final historyRows = await _db.query('workout_history'); final settingsRows = await _db.query('settings'); + + if (historyRows.isEmpty) { + final existing = await BackupService.instance.readBackup(); + final existingHistory = + (existing?['workout_history'] as List?) ?? const []; + if (existingHistory.isNotEmpty) { + debugPrint( + 'WorkoutApp: REFUSING to overwrite the backup — this DB has no ' + 'workout history but the backup holds ${existingHistory.length} ' + 'session(s). A restore almost certainly failed; keeping the backup ' + 'so the data stays recoverable.', + ); + return; + } + } + await BackupService.instance.export({ 'exercise_state': exerciseRows, 'workout_history': historyRows, @@ -514,7 +624,16 @@ class StorageService { if (hasHistory > 0 || hasType != null) return; // DB has real data final backup = await BackupService.instance.readBackup(); - if (backup == null) return; + if (backup == null) { + // readBackup() has already said which case this is. Say what it MEANS: + // the DB is sitting at seeded defaults and that is now the live state. + debugPrint( + 'WorkoutApp: no backup restored — the database is at factory defaults. ' + 'If this is a reinstall, progression state (weights, reps, streaks) ' + 'has been lost and must be re-entered.', + ); + return; + } await _db.transaction((txn) async { for (final row in (backup['exercise_state'] as List? ?? []) diff --git a/stronglift_replacement/workout_app/pubspec.lock b/stronglift_replacement/workout_app/pubspec.lock index c6b06bc..8c56cb1 100644 --- a/stronglift_replacement/workout_app/pubspec.lock +++ b/stronglift_replacement/workout_app/pubspec.lock @@ -141,11 +141,11 @@ packages: dependency: "direct main" description: path: crdt_sync_dart - ref: "crdt_sync_dart-v0.7.0" - resolved-ref: "6eef99d66132e12eb5056ae8e1780b34237fa013" + ref: "crdt_sync_dart-v0.9.0" + resolved-ref: f902bbe0aa6689adbe4ef0ef6bb298655f4e136a url: "https://github.com/kuhyx/utils" source: git - version: "0.7.0" + version: "0.9.0" crypto: dependency: transitive description: diff --git a/stronglift_replacement/workout_app/pubspec.yaml b/stronglift_replacement/workout_app/pubspec.yaml index a3ef5d0..06b4fc0 100644 --- a/stronglift_replacement/workout_app/pubspec.yaml +++ b/stronglift_replacement/workout_app/pubspec.yaml @@ -12,7 +12,7 @@ dependencies: crdt_sync: git: url: https://github.com/kuhyx/utils - ref: crdt_sync_dart-v0.7.0 + ref: crdt_sync_dart-v0.9.0 path: crdt_sync_dart flutter: sdk: flutter 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 90e5d64..802d5b2 100644 --- a/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart +++ b/stronglift_replacement/workout_app/test/screens/settings_screen_test.dart @@ -134,14 +134,40 @@ void main() { expect(find.text('Settings'), findsOneWidget); }); - testWidgets('shows WEIGHTS and PROGRESSION THRESHOLDS sections', ( + testWidgets('shows WEIGHTS, TARGET REPS and PROGRESSION THRESHOLDS', ( tester, ) async { await _pump(tester, _wrap()); expect(find.text('WEIGHTS'), findsOneWidget); + // TARGET REPS and the thresholds below it are off-screen at the test + // viewport height, so scroll them into view rather than asserting on + // whatever happens to be built. + await tester.scrollUntilVisible(find.text('TARGET REPS'), 200); + expect(find.text('TARGET REPS'), findsOneWidget); + await tester.scrollUntilVisible(find.text('PROGRESSION THRESHOLDS'), 200); expect(find.text('PROGRESSION THRESHOLDS'), findsOneWidget); }); + testWidgets('increment reps button increases the target reps', ( + tester, + ) async { + await _pump(tester, _wrap()); + await tester.scrollUntilVisible(find.text('TARGET REPS'), 200); + await tester.pumpAndSettle(); + // Situp defaults to 30 reps and is the only exercise at that value. + expect(find.text('30 reps'), findsOneWidget); + final plus = find.descendant( + of: find.ancestor( + of: find.text('30 reps'), + matching: find.byType(Row), + ).first, + matching: find.byIcon(Icons.add), + ); + await tester.tap(plus); + await tester.pumpAndSettle(); + expect(find.text('31 reps'), findsOneWidget); + }); + testWidgets('shows all exercise names from both workout plans', ( tester, ) async { @@ -193,6 +219,10 @@ void main() { testWidgets('threshold circles show values 1-5', (tester) async { await _pump(tester, _wrap()); + // The threshold cards sit below WEIGHTS and TARGET REPS, past the test + // viewport's fold, so they are not built until scrolled to. + await tester.scrollUntilVisible(find.text('PROGRESSION THRESHOLDS'), 200); + await tester.pumpAndSettle(); for (int i = 1; i <= 5; i++) { expect(find.text('$i'), findsWidgets); } @@ -582,6 +612,11 @@ void main() { testWidgets('changing a success threshold persists it', (tester) async { await _pump(tester, _wrap()); + // Scroll to the section header first: the threshold rows below it are not + // built at all until then, so a `.first` finder for the label would throw + // "Bad state: No element" before it could match anything. + await tester.scrollUntilVisible(find.text('PROGRESSION THRESHOLDS'), 200); + await tester.pumpAndSettle(); final label = find.text('↑ Increase after N successes').first; await tester.scrollUntilVisible( label, @@ -630,6 +665,33 @@ void main() { expect(states2!.any((s) => s.failThreshold == 4), isTrue); }); + testWidgets('reps change is debounced then written to storage', ( + tester, + ) async { + await _pump(tester, _wrap()); + await tester.scrollUntilVisible(find.text('TARGET REPS'), 200); + await tester.pumpAndSettle(); + + // Situp defaults to 30 reps and is the only exercise at that value. + final minus = find.descendant( + of: find + .ancestor(of: find.text('30 reps'), matching: find.byType(Row)) + .first, + matching: find.byIcon(Icons.remove), + ); + // Same 600ms debounce as weights: run on the real loop so the timer fires. + await tester.runAsync(() async { + await tester.tap(minus); + await Future.delayed(const Duration(milliseconds: 800)); + }); + await tester.pump(); + + final after = (await tester.runAsync( + () => StorageService.instance.getExerciseState('Situp'), + ))!.reps; + expect(after, 29, reason: 'the decrement must reach storage'); + }); + testWidgets('weight change is debounced then written to storage', ( tester, ) async { diff --git a/stronglift_replacement/workout_app/test/services/storage_service_test.dart b/stronglift_replacement/workout_app/test/services/storage_service_test.dart index 8b7077c..60f498e 100644 --- a/stronglift_replacement/workout_app/test/services/storage_service_test.dart +++ b/stronglift_replacement/workout_app/test/services/storage_service_test.dart @@ -1,3 +1,4 @@ +import 'dart:convert'; import 'dart:io'; import 'package:flutter_test/flutter_test.dart'; @@ -156,6 +157,25 @@ void main() { }); }); + group('setExerciseReps', () { + test('updates reps and resets streaks', () async { + // Progression can only ever RAISE reps, so this is the only way to + // correct a rep target a defaults re-seed set wrong. + const name = 'Situp'; + await _svc.setExerciseReps(name, 31); + final state = await _svc.getExerciseState(name); + expect(state!.reps, 31); + expect(state.successStreak, 0); + expect(state.failStreak, 0); + }); + + test('can lower a rep target', () async { + const name = 'Situp'; + await _svc.setExerciseReps(name, 25); + expect((await _svc.getExerciseState(name))!.reps, 25); + }); + }); + group('setExerciseWeight', () { test('updates weight and resets streaks', () async { final name = workoutA.first.name; @@ -438,6 +458,169 @@ void main() { // ── Restore from backup ───────────────────────────────────────────────────── + // Regression guard for the 2026-08-05 data loss: a reinstall re-seeded the + // DB to factory defaults, and the first write afterwards exported those + // defaults over /sdcard/WorkoutTracker/backup.json -- destroying the only + // off-device copy of months of progression. The seed is survivable; the + // clobber is what made it permanent. + group('restoreSyncedSessions', () { + Map session(String date, String start) => { + 'workout_type': 'B', + 'date': date, + 'start_time': start, + 'duration_seconds': 3600, + 'succeeded': true, + 'exercises': [ + {'name': 'Situp', 'targetReps': 31, 'targetWeight': 10.0, 'sets': []}, + ], + }; + + test('restores sessions the local DB is missing', () async { + final restored = await _svc.restoreSyncedSessions([ + session('2026-07-17', '2026-07-17T09:48:34.338'), + session('2026-07-27', '2026-07-27T09:14:22.547'), + ]); + expect(restored, 2); + final hist = await _svc.getWorkoutHistory(); + expect(hist.length, 2); + expect(hist.first['date'], '2026-07-27'); + }); + + test('is idempotent — re-syncing never duplicates a session', () async { + final payloads = [session('2026-07-17', '2026-07-17T09:48:34.338')]; + expect(await _svc.restoreSyncedSessions(payloads), 1); + expect(await _svc.restoreSyncedSessions(payloads), 0); + expect((await _svc.getWorkoutHistory()).length, 1); + }); + + test('does not duplicate a session the device already recorded', () async { + final payload = session('2026-08-10', '2026-08-10T10:19:44.075703'); + await _svc.saveSession( + date: '2026-08-10', + workoutType: 'B', + durationSeconds: 10313, + succeeded: true, + json: jsonEncode(payload), + ); + expect(await _svc.restoreSyncedSessions([payload]), 0); + expect((await _svc.getWorkoutHistory()).length, 1); + }); + + test('ignores PC-side records that are not sessions', () async { + // runnerup/manual entries have no `exercises` and belong in the synced + // list, not local history. + final restored = await _svc.restoreSyncedSessions([ + {'kind': 'runnerup_verified', 'date': '2026-07-12', 'type': 'x'}, + {'kind': 'manual_workout', 'date': '2026-07-13', 'type': 'y'}, + ]); + expect(restored, 0); + expect(await _svc.getWorkoutHistory(), isEmpty); + }); + + test('empty payload list is a no-op', () async { + expect(await _svc.restoreSyncedSessions([]), 0); + }); + + test('skips a payload with no date or start_time', () async { + final restored = await _svc.restoreSyncedSessions([ + {'exercises': [], 'duration_seconds': 60}, + ]); + expect(restored, 0); + }); + + test('a corrupt local row does not block restoring the rest', () async { + await _svc.saveSession( + date: '2026-07-01', + workoutType: 'A', + durationSeconds: 60, + succeeded: true, + json: 'not valid json', + ); + final restored = await _svc.restoreSyncedSessions([ + session('2026-07-17', '2026-07-17T09:48:34.338'), + ]); + expect(restored, 1); + }); + }); + + group('_backupNow clobber guard', () { + late Directory tmp; + + setUp(() { + tmp = Directory.systemTemp.createTempSync('mw_clobber'); + BackupService.baseDirForTesting = tmp.path; + }); + + tearDown(() { + BackupService.baseDirForTesting = kBackupDir; + tmp.deleteSync(recursive: true); + }); + + test('refuses to overwrite a real backup from a history-less DB', () async { + // A good backup: real progression and a real session. + await BackupService.instance.export({ + 'exercise_state': [ + { + 'name': 'Situp', + 'weight': 10.0, + 'reps': 31, + 'success_streak': 2, + 'fail_streak': 0, + 'max_weight': 10.0, + 'success_threshold': 5, + 'fail_threshold': 2, + }, + ], + 'workout_history': [ + { + 'date': '2026-07-17', + 'workout_type': 'B', + 'duration_seconds': 6465, + 'succeeded': 1, + 'json': '{}', + }, + ], + 'settings': [ + {'key': 'last_workout_type', 'value': 'B'}, + ], + }); + + // The DB here is freshly seeded: defaults, and no history at all. + // Any write triggers _backupNow(), which used to export over the good + // backup. setExerciseWeight is one such write. + await _svc.setExerciseWeight('Situp', 10.0); + // Let the unawaited _backupNow() settle. + await Future.delayed(const Duration(milliseconds: 50)); + + final backup = await BackupService.instance.readBackup(); + final history = backup!['workout_history'] as List; + final exercises = backup['exercise_state'] as List; + + // The good backup must still be intact. + expect(history.length, 1, reason: 'real session was clobbered'); + expect((history.first as Map)['date'], '2026-07-17'); + final situp = exercises.firstWhere( + (e) => (e as Map)['name'] == 'Situp', + ) as Map; + expect(situp['reps'], 31, reason: 'progression reps were clobbered'); + }); + + test('still writes the backup once the DB has real history', () async { + await _svc.saveSession( + date: '2026-08-10', + workoutType: 'B', + durationSeconds: 10313, + succeeded: true, + json: '{}', + ); + await Future.delayed(const Duration(milliseconds: 50)); + + final backup = await BackupService.instance.readBackup(); + expect(backup, isNotNull, reason: 'a real DB must still back itself up'); + expect((backup!['workout_history'] as List).length, 1); + }); + }); + group('restoreFromBackupIfNeeded', () { late Directory tmp; diff --git a/workout-sync.service b/workout-sync.service new file mode 100644 index 0000000..4c1c225 --- /dev/null +++ b/workout-sync.service @@ -0,0 +1,15 @@ +[Unit] +Description=Pull phone workouts into the screen-locker log (crdt_sync) + +[Service] +Type=oneshot +# No WorkingDirectory/PYTHONPATH needed: screen_locker is pip-installed +# (pip install --user --break-system-packages -e .), so /usr/bin/python3 +# finds it via user site-packages. Run as a module for the same reason the +# fit check is (see dd233b2) -- it works where nothing is installed. +# +# --sync-only is headless: no DISPLAY, no Tk, no lock screen. It pulls other +# devices' workouts and applies their credit, which the locker itself only +# did once per process start -- so a workout finished after login was invisible +# until the next login. +ExecStart=/usr/bin/python3 -m screen_locker.screen_lock --sync-only diff --git a/workout-sync.timer b/workout-sync.timer new file mode 100644 index 0000000..8221e24 --- /dev/null +++ b/workout-sync.timer @@ -0,0 +1,12 @@ +[Unit] +Description=Periodically pull phone workouts into the screen-locker log + +[Timer] +# Every 15 minutes, matching diet-guard-sync.timer and the phone app's +# WorkManager floor, so both sides converge on roughly the same cadence. +# Persistent catches a run missed while the machine was suspended. +OnCalendar=*-*-* *:00/15:00 +Persistent=true + +[Install] +WantedBy=timers.target