diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8e75ccd..e011b84 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -102,7 +102,7 @@ repos: - pytest - requests - gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock - - crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.5.1#subdirectory=crdt-sync + - crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync - mcp>=2,<3 - pydantic # Test suites intentionally use patterns (protected-access, magic-value diff --git a/CLAUDE.md b/CLAUDE.md index a18d913..eb00270 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -178,9 +178,29 @@ Consequences worth knowing before touching `app/`: lives in that profile: changing either hides the entire local food log. - **The wrapper holds the GitHub token**, not the browser, and proxies `api.github.com` plus the CORS-less device flow (`lib/desktop/github_proxy.dart`). -- **The desktop pushes as device id `desktop`**, the phone as `phone`, the - Python side as `pc`. Sharing an id means two devices overwrite each other's - pushed log every tick. +- **Every device pushes under a persisted per-install uuid**, not the old + fixed `pc`/`phone`/`desktop` role constants. A role constant collides the + moment a second machine takes the same role, and a reinstall silently + inherits the previous install's CRDT identity. The id lives in + `~/.local/share/diet_guard/.device_id` on the PC (`diet_guard/_device.py`) + and under SharedPreferences `crdt.nodeId` in the app + (`lib/services/sync_device_id.dart`), the same key `todo` and + `home_inventory` use. + - The old role constant survives as the **legacy id** + (`SYNC_LEGACY_DEVICE_ID` / `legacySyncDeviceId`), passed to `sync_log` / + `syncLog` so `devices//` is skipped as *this device's own*. Drop + it to `None`/null only after that path has been deleted — until then, + removing it makes every tick re-download and re-merge this device's own + pre-migration log as though a peer wrote it. + - The app resolves its id **asynchronously at startup** + (`initSyncDeviceId()` in `main.dart`, before `LogStorageService.init()`). + HLC stamping is synchronous, so `currentSyncDeviceId` falls back to the + compile-time role constant until that completes: a caller that stamps too + early gets a valid-but-pre-migration id rather than a crash. Don't move + that call later. + - **Tests must redirect `_device.SYNC_DEVICE_ID_FILE`** (conftest already + does). Without it a test mints a uuid into the real state dir and this + machine's live sync identity is decided by whichever test ran first. - **Desktop reminders only exist while the window is open.** A browser has no WorkManager equivalent; the PC's real backstop is `diet-guard-gate.timer`, which locks the screen. Don't "fix" this with a background service without diff --git a/app/lib/main.dart b/app/lib/main.dart index 31095ed..95be649 100644 --- a/app/lib/main.dart +++ b/app/lib/main.dart @@ -2,8 +2,8 @@ /// background due-slot check, then shows the primary meal-logging screen. library; -import 'dart:async'; +import 'dart:async'; import 'package:diet_guard_app/screens/log_meal_screen.dart'; import 'package:diet_guard_app/services/app_settings_service.dart'; import 'package:diet_guard_app/services/background_tasks.dart'; @@ -12,11 +12,15 @@ import 'package:diet_guard_app/services/foodbank_service.dart'; import 'package:diet_guard_app/services/frame_stats.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; import 'package:diet_guard_app/services/notification_service.dart'; +import 'package:diet_guard_app/services/sync_device_id.dart'; import 'package:diet_guard_app/ui/theme.dart'; import 'package:flutter/material.dart'; Future main() async { WidgetsFlutterBinding.ensureInitialized(); + // Before anything that stamps an Hlc or syncs: until this resolves, the + // device id falls back to the pre-migration role constant. + await initSyncDeviceId(); await LogStorageService.init(); await AppSettingsService.init(); await BudgetHistoryService.init(); diff --git a/app/lib/services/sync_device_id.dart b/app/lib/services/sync_device_id.dart new file mode 100644 index 0000000..eb6c04b --- /dev/null +++ b/app/lib/services/sync_device_id.dart @@ -0,0 +1,55 @@ +/// This install's persisted sync device id. +library; + +import 'package:diet_guard_app/services/github_client_factory.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:uuid/uuid.dart'; + +/// SharedPreferences key holding this install's device id. +/// +/// The same key the sibling apps (`todo`, `home_inventory`, `wake_alarm`) +/// use for the same purpose. +const kSyncDeviceIdKey = 'crdt.nodeId'; + +String? _cached; + +/// This install's sync device id, for HLC stamping and the pushed path. +/// +/// Synchronous because [Hlc] stamping is synchronous and threading a future +/// through every merge adapter would be a large change for no benefit. Call +/// [initSyncDeviceId] once during startup, before any sync or merge runs; +/// until then this falls back to the compile-time role constant +/// ([syncDeviceId]) so a caller that forgets still writes a *valid* id rather +/// than crashing -- it just writes the pre-migration one. +String get currentSyncDeviceId => _cached ?? syncDeviceId; + +/// The id this install pushed under before migrating to a persisted uuid. +/// +/// Passed to `syncLog` as its legacy id so the log already sitting at +/// `devices//` is treated as this device's own rather than a peer's; +/// without it every tick re-downloads and re-merges this device's own +/// pre-migration history. Set to null once that path has been reclaimed. +const String legacySyncDeviceId = syncDeviceId; + +/// Loads (or mints and persists) this install's device id. +/// +/// A per-install uuid rather than the fixed `phone`/`desktop` role constant: +/// two installs sharing an id overwrite each other's pushed file on every +/// tick, and a reinstall would inherit the previous install's CRDT identity. +/// +/// Idempotent -- safe to call more than once. +Future initSyncDeviceId({SharedPreferences? prefs}) async { + final store = prefs ?? await SharedPreferences.getInstance(); + final existing = store.getString(kSyncDeviceIdKey); + if (existing != null && existing.isNotEmpty) { + _cached = existing; + return existing; + } + final minted = const Uuid().v4(); + await store.setString(kSyncDeviceIdKey, minted); + _cached = minted; + return minted; +} + +/// Resets the cached id. Test-only. +void resetSyncDeviceIdForTest() => _cached = null; diff --git a/app/lib/services/sync_merge.dart b/app/lib/services/sync_merge.dart index 224d4ef..69a1b74 100644 --- a/app/lib/services/sync_merge.dart +++ b/app/lib/services/sync_merge.dart @@ -29,15 +29,15 @@ /// adapters field-for-field. library; -import 'dart:convert'; +import 'dart:convert'; import 'package:crdt_sync/crdt_sync.dart'; import 'package:crypto/crypto.dart'; import 'package:diet_guard_app/models/food_bank_record.dart'; import 'package:diet_guard_app/models/food_entry.dart'; import 'package:diet_guard_app/services/budget_schedule.dart'; -import 'package:diet_guard_app/services/github_client_factory.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; +import 'package:diet_guard_app/services/sync_device_id.dart'; /// Derives a deterministic [Hlc] for [entry] from its own `time` field. /// @@ -50,7 +50,7 @@ import 'package:diet_guard_app/services/log_storage_service.dart'; /// survives a merge. Hlc entryHlc(FoodEntry entry) { final wallTimeMs = DateTime.tryParse(entry.time)?.millisecondsSinceEpoch ?? 0; - return Hlc.newTick(syncDeviceId, wallTimeMsOverride: wallTimeMs); + return Hlc.newTick(currentSyncDeviceId, wallTimeMsOverride: wallTimeMs); } /// Deterministic id for a pre-`id` legacy entry, from `(time, desc)`. @@ -206,7 +206,7 @@ Log foodBankToLog(Map bank) { 'body': ( mapEntry.value.toJson(), Hlc.newTick( - syncDeviceId, + currentSyncDeviceId, wallTimeMsOverride: mapEntry.value.count.toInt(), ), ), @@ -270,7 +270,7 @@ Log manualBankToLog(Map bank) { fields: { 'body': ( body, - Hlc.newTick(syncDeviceId, wallTimeMsOverride: wallTimeMs), + Hlc.newTick(currentSyncDeviceId, wallTimeMsOverride: wallTimeMs), ), }, ); @@ -337,7 +337,7 @@ Hlc budgetHlc(Map record) { record['t'] as String? ?? '', )?.toUtc().millisecondsSinceEpoch ?? 0; - return Hlc.newTick(syncDeviceId, wallTimeMsOverride: wallTimeMs); + return Hlc.newTick(currentSyncDeviceId, wallTimeMsOverride: wallTimeMs); } /// Field-name prefix for the effective-from budget history, one field per @@ -355,7 +355,7 @@ const budgetHistoryFieldPrefix = 'hist:'; Hlc historyHlc(BudgetEntry entry) { final wallTimeMs = DateTime.tryParse(entry.editedAt)?.toUtc().millisecondsSinceEpoch ?? 0; - return Hlc.newTick(syncDeviceId, wallTimeMsOverride: wallTimeMs); + return Hlc.newTick(currentSyncDeviceId, wallTimeMsOverride: wallTimeMs); } /// Converts a raw local budget record plus its history into a [Log]. diff --git a/app/lib/services/sync_service.dart b/app/lib/services/sync_service.dart index 7243fbc..c4fe02f 100644 --- a/app/lib/services/sync_service.dart +++ b/app/lib/services/sync_service.dart @@ -17,8 +17,8 @@ import 'package:diet_guard_app/models/food_entry.dart'; import 'package:diet_guard_app/services/app_settings_service.dart'; import 'package:diet_guard_app/services/budget_history_service.dart'; import 'package:diet_guard_app/services/foodbank_service.dart'; -import 'package:diet_guard_app/services/github_client_factory.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; +import 'package:diet_guard_app/services/sync_device_id.dart'; import 'package:diet_guard_app/services/sync_merge.dart'; import 'package:diet_guard_app/services/sync_state_factory.dart'; @@ -35,7 +35,8 @@ Future runSync(RemoteStore client, {SyncStateStore? stateStore}) async { final mergedLog = await syncLog( client: client, - deviceId: syncDeviceId, + deviceId: currentSyncDeviceId, + legacyDeviceId: legacySyncDeviceId, pathPrefix: _devicesDir, localLog: dayLogToLog(local), encode: encodeLogForPush, @@ -67,7 +68,8 @@ Future runSync(RemoteStore client, {SyncStateStore? stateStore}) async { Future _syncFoodBank(RemoteStore client) async { final merged = await syncLog( client: client, - deviceId: syncDeviceId, + deviceId: currentSyncDeviceId, + legacyDeviceId: legacySyncDeviceId, pathPrefix: _devicesDir, localLog: foodBankToLog(await FoodBankService.instance.readBank()), encode: encodeFoodBankForPush, @@ -88,7 +90,8 @@ Future _syncManualBank(RemoteStore client) async { final local = await FoodBankService.instance.readManualBank(); final merged = await syncLog( client: client, - deviceId: syncDeviceId, + deviceId: currentSyncDeviceId, + legacyDeviceId: legacySyncDeviceId, pathPrefix: _devicesDir, localLog: manualBankToLog(local), encode: encodeManualBankForPush, @@ -120,7 +123,8 @@ Future _syncBudget(RemoteStore client) async { final mergedBudgetLog = await syncLog( client: client, - deviceId: syncDeviceId, + deviceId: currentSyncDeviceId, + legacyDeviceId: legacySyncDeviceId, pathPrefix: _devicesDir, localLog: budgetToLog( localRecord, diff --git a/app/pubspec.lock b/app/pubspec.lock index 6617784..386fc70 100644 --- a/app/pubspec.lock +++ b/app/pubspec.lock @@ -85,11 +85,11 @@ packages: dependency: "direct main" description: path: crdt_sync_dart - ref: "crdt_sync_dart-v0.6.0" - resolved-ref: fc99eff00599a4c3195e4c1573ba08f15909a10d + ref: "crdt_sync_dart-v0.7.0" + resolved-ref: "6eef99d66132e12eb5056ae8e1780b34237fa013" url: "https://github.com/kuhyx/utils" source: git - version: "0.6.0" + version: "0.7.0" crypto: dependency: "direct main" description: diff --git a/app/pubspec.yaml b/app/pubspec.yaml index 0697370..42d2d1a 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -11,7 +11,7 @@ dependencies: crdt_sync: git: url: https://github.com/kuhyx/utils - ref: crdt_sync_dart-v0.6.0 + ref: crdt_sync_dart-v0.7.0 path: crdt_sync_dart crypto: ^3.0.6 flutter: diff --git a/app/test/services/sync_device_id_test.dart b/app/test/services/sync_device_id_test.dart new file mode 100644 index 0000000..3df55cf --- /dev/null +++ b/app/test/services/sync_device_id_test.dart @@ -0,0 +1,73 @@ +import 'package:diet_guard_app/services/github_client_factory.dart'; +import 'package:diet_guard_app/services/sync_device_id.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:uuid/uuid.dart'; + +void main() { + setUp(() { + SharedPreferences.setMockInitialValues({}); + resetSyncDeviceIdForTest(); + }); + + group('initSyncDeviceId', () { + test('mints and persists a uuid on first run', () async { + final minted = await initSyncDeviceId(); + + expect(Uuid.isValidUUID(fromString: minted), isTrue); + final prefs = await SharedPreferences.getInstance(); + expect(prefs.getString(kSyncDeviceIdKey), minted); + }); + + test('returns the same id on every later run', () async { + final first = await initSyncDeviceId(); + resetSyncDeviceIdForTest(); + + expect(await initSyncDeviceId(), first); + }); + + test('adopts an id already stored under the shared key', () async { + SharedPreferences.setMockInitialValues({ + kSyncDeviceIdKey: 'pre-existing-id', + }); + + expect(await initSyncDeviceId(), 'pre-existing-id'); + }); + + test('treats a blank stored id as absent', () async { + SharedPreferences.setMockInitialValues({kSyncDeviceIdKey: ''}); + + expect(await initSyncDeviceId(), isNotEmpty); + }); + + test('is idempotent', () async { + final first = await initSyncDeviceId(); + + expect(await initSyncDeviceId(), first); + }); + }); + + group('currentSyncDeviceId', () { + test('falls back to the role constant before init runs', () { + // A caller that stamps an Hlc before startup finishes must still get a + // valid id -- the pre-migration one -- rather than crash. + expect(currentSyncDeviceId, syncDeviceId); + }); + + test('returns the persisted uuid once init has run', () async { + final minted = await initSyncDeviceId(); + + expect(currentSyncDeviceId, minted); + expect(currentSyncDeviceId, isNot(syncDeviceId)); + }); + }); + + group('legacySyncDeviceId', () { + test('is the role constant this build shipped with', () { + // Passed to syncLog so devices// is skipped as this device's + // own; if it ever drifted from the pre-migration value, the old log + // would be re-merged as a peer's on every tick. + expect(legacySyncDeviceId, syncDeviceId); + }); + }); +} diff --git a/diet_guard/_constants.py b/diet_guard/_constants.py index 625c057..9b5995c 100644 --- a/diet_guard/_constants.py +++ b/diet_guard/_constants.py @@ -88,7 +88,17 @@ GATE_LOCK_FILE: Path = DATA_DIR / ".gate.lock" # happens client-side (see _sync_merge.py), never via git. SYNC_REPO_OWNER: str = "kuhyx" SYNC_REPO_NAME: str = "syncs" -SYNC_DEVICE_ID: str = "pc" +# The id this machine pushed under before per-install uuids. The sync reader +# still treats it as this device's own, so the log pushed under it is not +# pulled back and re-merged as a peer's; drop to None once devices/pc/ has +# been reclaimed. See SYNC_DEVICE_ID_FILE. +SYNC_LEGACY_DEVICE_ID: str | None = "pc" +# This device's sync id, minted once and persisted. A fixed "pc" collides the +# moment a second machine takes the same role, and after a reinstall the new +# install would inherit the old one's CRDT identity; a uuid makes both +# impossible by construction. Lives beside the other state, not in ~/.config, +# because losing it means being seen as a brand-new device. +SYNC_DEVICE_ID_FILE: Path = DATA_DIR / ".device_id" # A fine-grained GitHub PAT, scoped to just SYNC_REPO_NAME's contents. The # user creates this once via github.com (see CLAUDE.md) and saves it here, # mode 600. Never committed -- this path is outside the repo entirely. diff --git a/diet_guard/_device.py b/diet_guard/_device.py new file mode 100644 index 0000000..17286c8 --- /dev/null +++ b/diet_guard/_device.py @@ -0,0 +1,28 @@ +"""This machine's persisted sync identity. + +Its own module rather than living in :mod:`diet_guard._sync`, because +:mod:`diet_guard._sync_merge` needs it too for HLC stamping and importing +``_sync`` from there would be a cycle. +""" + +from __future__ import annotations + +from crdt_sync import DeviceIdentity, load_device_identity + +from diet_guard._constants import SYNC_DEVICE_ID_FILE, SYNC_LEGACY_DEVICE_ID + + +def device_identity() -> DeviceIdentity: + """Return this machine's sync identity, minting it on first use. + + Deliberately not cached at import time: the id file is redirected + per-test, and a module-level constant would freeze whichever value the + first import happened to see. Reading a small file once per sync tick is + cheaper than that surprise. + """ + return load_device_identity(SYNC_DEVICE_ID_FILE, legacy_id=SYNC_LEGACY_DEVICE_ID) + + +def device_id() -> str: + """Return just this machine's current sync id, for HLC stamping.""" + return device_identity().device_id diff --git a/diet_guard/_sync.py b/diet_guard/_sync.py index ddf6979..1160215 100644 --- a/diet_guard/_sync.py +++ b/diet_guard/_sync.py @@ -22,6 +22,7 @@ import logging from crdt_sync import ( CONFIG_FILE, ConfigError, + DeviceIdentity, FileSyncStateStore, FirebaseAuthError, GitHubSyncClient, @@ -42,13 +43,13 @@ from diet_guard._budget_history import ( write_raw_history, ) from diet_guard._constants import ( - SYNC_DEVICE_ID, SYNC_REPO_NAME, SYNC_REPO_OWNER, SYNC_STATE_FILE, SYNC_TIMEOUT_SECONDS, SYNC_TOKEN_FILE, ) +from diet_guard._device import device_identity from diet_guard._foodbank import read_food_bank, rebuild_food_bank, write_food_bank from diet_guard._foodbank_manual import read_manual_bank, write_manual_bank from diet_guard._state import DayLog, read_raw_log, resign_entry, write_raw_log @@ -172,6 +173,7 @@ def _pull_remote_logs( remote_revs: dict[str, str], state: SyncState, seen_revs: dict[str, str], + identity: DeviceIdentity, ) -> list[Log]: """Return every other device's last-pushed log, skipping this one. @@ -188,7 +190,7 @@ def _pull_remote_logs( """ remote_logs: list[Log] = [] for device_id in client.list_directory(_DEVICES_DIR): - if device_id == SYNC_DEVICE_ID: + if identity.is_own(device_id): continue remote_rev = remote_revs.get(device_id) if remote_rev is not None and remote_rev == state.peer_revs.get(device_id): @@ -231,10 +233,11 @@ def _sync_food_bank(client: GitHubSyncClient) -> None: contains fixes that, and stays identical across devices because that rebuild comes from the *merged* log both devices share. """ + identity = device_identity() local = read_food_bank() merged = food_bank_to_log(local) for device_id in client.list_directory(_DEVICES_DIR): - if device_id == SYNC_DEVICE_ID: + if identity.is_own(device_id): continue text = client.get_file_text(_device_food_bank_path(device_id)) if text is None: @@ -257,7 +260,7 @@ def _sync_food_bank(client: GitHubSyncClient) -> None: write_food_bank(resolved) merged = food_bank_to_log(resolved) client.put_file_text( - _device_food_bank_path(SYNC_DEVICE_ID), + _device_food_bank_path(identity.device_id), json.dumps( {record_id: record.to_dict() for record_id, record in merged.items()}, indent=2, @@ -274,9 +277,10 @@ def _sync_manual_bank(client: GitHubSyncClient) -> None: ``food_bank.json`` they need a real merge: last-writer-wins per food name by edit time, union across devices. """ + identity = device_identity() merged = manual_bank_to_log(read_manual_bank()) for device_id in client.list_directory(_DEVICES_DIR): - if device_id == SYNC_DEVICE_ID: + if identity.is_own(device_id): continue text = client.get_file_text(_device_manual_bank_path(device_id)) if text is None: @@ -295,7 +299,7 @@ def _sync_manual_bank(client: GitHubSyncClient) -> None: return write_manual_bank(log_to_manual_bank(merged)) client.put_file_text( - _device_manual_bank_path(SYNC_DEVICE_ID), + _device_manual_bank_path(identity.device_id), json.dumps( {record_id: record.to_dict() for record_id, record in merged.items()}, indent=2, @@ -315,9 +319,10 @@ def _sync_budget(client: GitHubSyncClient) -> None: to the merge nor overwrites a real budget pulled from elsewhere, and if *no* device has ever set one, nothing is written or pushed. """ + identity = device_identity() merged = budget_to_log(read_raw_record(), load_entries()) for device_id in client.list_directory(_DEVICES_DIR): - if device_id == SYNC_DEVICE_ID: + if identity.is_own(device_id): continue text = client.get_file_text(_device_budget_path(device_id)) if text is None: @@ -347,7 +352,7 @@ def _sync_budget(client: GitHubSyncClient) -> None: indent=2, ) client.put_file_text( - _device_budget_path(SYNC_DEVICE_ID), + _device_budget_path(identity.device_id), push_json, message="diet_guard sync", ) @@ -382,13 +387,16 @@ def run_sync() -> DayLog: ) ) + identity = device_identity() state_store = FileSyncStateStore(SYNC_STATE_FILE) state = state_store.load() remote_revs = _remote_revisions(client) seen_revs: dict[str, str] = {} merged = daylog_to_log(read_raw_log()) - for remote_log in _pull_remote_logs(client, remote_revs, state, seen_revs): + for remote_log in _pull_remote_logs( + client, remote_revs, state, seen_revs, identity + ): merged = merge_logs(merged, remote_log) merged_daylog = log_to_daylog(merged) @@ -410,14 +418,14 @@ def run_sync() -> DayLog: revision = revision_of(push_json) if revision != state.pushed_rev: client.put_file_text( - _device_log_path(SYNC_DEVICE_ID), + _device_log_path(identity.device_id), push_json, message="diet_guard sync", ) # Published after the log, never before: a peer that cached "seen rev # X" against a log it never received would skip it forever. client.put_file_text( - f"{_REVS_DIR}/{SYNC_DEVICE_ID}", + f"{_REVS_DIR}/{identity.device_id}", revision, message="diet_guard sync: revision", ) diff --git a/diet_guard/_sync_merge.py b/diet_guard/_sync_merge.py index e8ba294..e9d6d0c 100644 --- a/diet_guard/_sync_merge.py +++ b/diet_guard/_sync_merge.py @@ -42,7 +42,7 @@ from typing import TYPE_CHECKING from crdt_sync import Hlc, Record from diet_guard._budget_history import BudgetEntry -from diet_guard._constants import SYNC_DEVICE_ID +from diet_guard._device import device_id from diet_guard._foodbank_manual import record_edit_time if TYPE_CHECKING: @@ -80,7 +80,7 @@ def _entry_hlc(entry: dict[str, object]) -> Hlc: except ValueError: dt = _EPOCH wall_time_ms = int(dt.timestamp() * 1000) - return Hlc.new_tick(SYNC_DEVICE_ID, wall_time_ms=wall_time_ms) + return Hlc.new_tick(device_id(), wall_time_ms=wall_time_ms) def _legacy_entry_id(entry: dict[str, object]) -> str: @@ -229,7 +229,7 @@ def _budget_hlc(record: dict[str, object]) -> Hlc: except ValueError: dt = _EPOCH wall_time_ms = int(dt.timestamp() * 1000) - return Hlc.new_tick(SYNC_DEVICE_ID, wall_time_ms=wall_time_ms) + return Hlc.new_tick(device_id(), wall_time_ms=wall_time_ms) def _history_hlc(entry: BudgetEntry) -> Hlc: @@ -245,7 +245,7 @@ def _history_hlc(entry: BudgetEntry) -> Hlc: moment = datetime.fromisoformat(entry.edited_at) except ValueError: moment = _EPOCH - return Hlc.new_tick(SYNC_DEVICE_ID, wall_time_ms=int(moment.timestamp() * 1000)) + return Hlc.new_tick(device_id(), wall_time_ms=int(moment.timestamp() * 1000)) def budget_to_log( @@ -360,7 +360,7 @@ def food_bank_to_log(bank: Mapping[str, object]) -> Log: continue count = record.get("count") ticks = int(count) if isinstance(count, (int, float)) else 0 - hlc = Hlc.new_tick(SYNC_DEVICE_ID, wall_time_ms=ticks) + hlc = Hlc.new_tick(device_id(), wall_time_ms=ticks) log[name] = Record(id=name, fields={"body": (dict(record), hlc)}) return log @@ -405,7 +405,7 @@ def manual_bank_to_log(bank: Mapping[str, object]) -> Log: if not isinstance(record, dict): continue hlc = Hlc.new_tick( - SYNC_DEVICE_ID, + device_id(), wall_time_ms=_wall_time_ms(record_edit_time(record)), ) body = {k: v for k, v in record.items() if k != "t"} diff --git a/diet_guard/tests/conftest.py b/diet_guard/tests/conftest.py index 6d7c22b..6fb527a 100644 --- a/diet_guard/tests/conftest.py +++ b/diet_guard/tests/conftest.py @@ -120,6 +120,13 @@ def _isolate_state(tmp_path: Path) -> Iterator[None]: "diet_guard._sync.SYNC_STATE_FILE", tmp_path / "sync_state.json", ), + # Without this a test that syncs mints a uuid into the REAL + # ~/.local/share/diet_guard/.device_id, and this machine's live sync + # identity is decided by whichever test happened to run first. + patch( + "diet_guard._device.SYNC_DEVICE_ID_FILE", + tmp_path / ".device_id", + ), # `run_sync` reads this to decide whether to build a Firebase-primary # mirror. On a developer machine the real file exists, so without this # every sync test would sign in and push to the live database. diff --git a/diet_guard/tests/test_sync.py b/diet_guard/tests/test_sync.py index 3ac9f75..2c26da2 100644 --- a/diet_guard/tests/test_sync.py +++ b/diet_guard/tests/test_sync.py @@ -17,6 +17,7 @@ import pytest from diet_guard import _sync from diet_guard._budget import budget_weight, daily_budget, write_budget from diet_guard._budget_history import load_entries +from diet_guard._device import device_id, device_identity from diet_guard._estimator import Nutrition from diet_guard._foodbank import lookup_food from diet_guard._state import load_log, log_meal @@ -105,9 +106,9 @@ class TestRunSync: # device's revision -- published *after* the log, so a peer can never # cache "seen rev X" against a log it never received. assert pushed == [ - "diet-guard-sync/devices/pc/food_bank.json", - "diet-guard-sync/devices/pc/food_log.json", - "diet-guard-sync/revs/pc", + f"diet-guard-sync/devices/{device_id()}/food_bank.json", + f"diet-guard-sync/devices/{device_id()}/food_log.json", + f"diet-guard-sync/revs/{device_id()}", ] log_call = next( call @@ -252,7 +253,7 @@ class TestSyncBudget: with patch.object(_sync, "GitHubSyncClient", return_value=client): _sync.run_sync() pushed_paths = {call.args[0] for call in client.put_file_text.call_args_list} - assert "diet-guard-sync/devices/pc/budget.json" in pushed_paths + assert f"diet-guard-sync/devices/{device_id()}/budget.json" in pushed_paths def test_nothing_pushed_when_no_budget_ever_set(self) -> None: """An uninitialized device contributes nothing -- no push, no crash.""" @@ -261,7 +262,7 @@ class TestSyncBudget: with patch.object(_sync, "GitHubSyncClient", return_value=client): _sync.run_sync() pushed_paths = {call.args[0] for call in client.put_file_text.call_args_list} - assert "diet-guard-sync/devices/pc/budget.json" not in pushed_paths + assert f"diet-guard-sync/devices/{device_id()}/budget.json" not in pushed_paths def test_remote_only_budget_is_adopted_locally(self) -> None: """Only the phone has ever set a budget -- the PC adopts it.""" @@ -448,7 +449,9 @@ class TestPullSkipsUnchangedPeers: state = SyncState(pushed_rev=None, peer_revs={"phone": "rev-1"}) seen: dict[str, str] = {} - logs = _sync._pull_remote_logs(client, {"phone": "rev-1"}, state, seen) + logs = _sync._pull_remote_logs( + client, {"phone": "rev-1"}, state, seen, device_identity() + ) assert logs == [] assert seen == {"phone": "rev-1"} @@ -463,7 +466,9 @@ class TestPullSkipsUnchangedPeers: state = SyncState(pushed_rev=None, peer_revs={"phone": "rev-1"}) seen: dict[str, str] = {} - _sync._pull_remote_logs(client, {"phone": "rev-2"}, state, seen) + _sync._pull_remote_logs( + client, {"phone": "rev-2"}, state, seen, device_identity() + ) client.get_file_text.assert_called_once() assert seen == {"phone": "rev-2"} @@ -486,6 +491,8 @@ class TestNoOpPushSuppression: _sync.run_sync() pushed_second = [call.args[0] for call in client.put_file_text.call_args_list] - assert "diet-guard-sync/devices/pc/food_log.json" in pushed_first - assert "diet-guard-sync/devices/pc/food_log.json" not in pushed_second - assert "diet-guard-sync/revs/pc" not in pushed_second + assert f"diet-guard-sync/devices/{device_id()}/food_log.json" in pushed_first + assert ( + f"diet-guard-sync/devices/{device_id()}/food_log.json" not in pushed_second + ) + assert f"diet-guard-sync/revs/{device_id()}" not in pushed_second diff --git a/diet_guard/tests/test_sync_banks.py b/diet_guard/tests/test_sync_banks.py index 51771eb..319c74f 100644 --- a/diet_guard/tests/test_sync_banks.py +++ b/diet_guard/tests/test_sync_banks.py @@ -12,6 +12,7 @@ import json from unittest.mock import patch from diet_guard import _sync +from diet_guard._device import device_id from diet_guard._estimator import Nutrition from diet_guard._foodbank import lookup_food, read_food_bank from diet_guard._foodbank_manual import add_manual_entry, read_manual_bank @@ -33,7 +34,7 @@ class TestSyncFoodBank: with patch.object(_sync, "GitHubSyncClient", return_value=client): _sync.run_sync() pushed = [call.args[0] for call in client.put_file_text.call_args_list] - assert "diet-guard-sync/devices/pc/food_bank.json" in pushed + assert f"diet-guard-sync/devices/{device_id()}/food_bank.json" in pushed def test_a_food_the_phone_logged_reaches_the_gate(self) -> None: """The payoff: a food eaten on the phone autocompletes on the PC. @@ -141,7 +142,7 @@ class TestSyncFoodBank: with patch.object(_sync, "GitHubSyncClient", return_value=client): _sync.run_sync() pushed = [call.args[0] for call in client.put_file_text.call_args_list] - assert "diet-guard-sync/devices/pc/food_bank.json" not in pushed + assert f"diet-guard-sync/devices/{device_id()}/food_bank.json" not in pushed class TestSyncManualBank: @@ -162,7 +163,7 @@ class TestSyncManualBank: with patch.object(_sync, "GitHubSyncClient", return_value=client): _sync.run_sync() pushed = [call.args[0] for call in client.put_file_text.call_args_list] - assert "diet-guard-sync/devices/pc/food_bank_manual.json" in pushed + assert f"diet-guard-sync/devices/{device_id()}/food_bank_manual.json" in pushed def test_a_remote_curated_entry_is_adopted_locally(self) -> None: """The point of the whole feature: a food added on the phone is usable diff --git a/diet_guard/tests/test_sync_device_id.py b/diet_guard/tests/test_sync_device_id.py new file mode 100644 index 0000000..c5aec3a --- /dev/null +++ b/diet_guard/tests/test_sync_device_id.py @@ -0,0 +1,64 @@ +"""Tests for this machine's persisted sync device id. + +Split out of ``test_sync.py`` to keep it under the repo's 500-line cap. +Covers the migration from the fixed ``"pc"`` role constant to a per-install +uuid -- specifically that the old path is still recognised as this device's +own, so its pre-migration log is not re-merged as a peer's. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from crdt_sync import DeviceIdentity, SyncState + +from diet_guard import _sync +from diet_guard._device import device_id, device_identity + + +def _mock_client(*, devices: tuple[str, ...] = ()) -> MagicMock: + """Build a mock sync client that lists ``devices`` and holds no files.""" + client = MagicMock() + client.list_directory.return_value = list(devices) + client.get_file_text.side_effect = lambda _path: None + del client.get_string_map + return client + + +class TestPullSkipsOwnLegacyPath: + """The uuid migration leaves this device's old 'pc' log behind.""" + + def test_skips_the_devices_own_legacy_id(self) -> None: + """devices/pc/ is this machine's own history, not a peer's. + + Without the legacy id in the skip set, every tick would download and + re-merge the log this machine pushed before it migrated -- idempotent + under LWW, but hundreds of KB of pure waste, 96 times a day. + """ + client = _mock_client(devices=("pc", "phone")) + state = SyncState(pushed_rev=None, peer_revs={}) + seen: dict[str, str] = {} + + _sync._pull_remote_logs(client, {}, state, seen, device_identity()) + + fetched = [call.args[0] for call in client.get_file_text.call_args_list] + assert "diet-guard-sync/devices/pc/food_log.json" not in fetched + assert "diet-guard-sync/devices/phone/food_log.json" in fetched + + def test_pulls_the_legacy_path_once_it_is_disowned(self) -> None: + """Negative control: with no legacy id, devices/pc/ IS a peer. + + Proves the skip above is the legacy id's doing, not an artifact of + the fixture -- and documents what happens after the old path is + reclaimed and SYNC_LEGACY_DEVICE_ID drops to None. + """ + client = _mock_client(devices=("pc",)) + state = SyncState(pushed_rev=None, peer_revs={}) + seen: dict[str, str] = {} + + _sync._pull_remote_logs( + client, {}, state, seen, DeviceIdentity(device_id=device_id()) + ) + + fetched = [call.args[0] for call in client.get_file_text.call_args_list] + assert "diet-guard-sync/devices/pc/food_log.json" in fetched diff --git a/pyproject.toml b/pyproject.toml index 3b41ec9..ff5ce26 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ description = "Tkinter/systemd log-to-unlock meal gate with calorie/macro budget requires-python = ">=3.10" 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.5.1#subdirectory=crdt-sync", + "crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync", "requests>=2.31.0", ] diff --git a/requirements.txt b/requirements.txt index 30324bf..05d4340 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.5.1#subdirectory=crdt-sync +crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock mcp>=2,<3 mypy>=1.8.0