diet-guard/diet_guard/_sync_client.py
Krzysztof kuhy Rudnicki c7371d7e0d perf(gate): unfreeze "Fetch from sync" and bound the interactive timeout
The button ran a full sync tick inline on the Tk thread, so the fullscreen
lock was dead for its whole duration -- ~18-27s measured against the live
remote, and unbounded in the worst case.

Two changes, and the second is what makes the first cheap:

* the pull moves to a daemon worker thread, with the result marshalled back
  through a queue polled on the Tk thread. The worker touches no widget, by
  construction: Tcl is not thread-safe and that includes `after`. Polling
  also makes the completion path testable with no thread at all.
* the button now runs the *narrow* pull. Its question is identical to the
  gate's -- "did a peer log this slot?" -- and `_reconcile_after_fetch`
  reads only `due_slots()`, so the budget and food banks it used to fetch
  were never consulted here. The full tick still runs after the window
  closes. Measured 84ms against the live remote.

Narrowing also shrinks the write race from tens of seconds to imperceptible:
the pull persists its merge, so a meal logged mid-fetch could be overwritten
by a snapshot taken before it. A second click while a fetch is in flight is
now ignored, which is the reentrancy guard and the race fix in one.

That guard started out as a disabled button, which is how it surfaced a
latent bug worth recording: the fetch button widget is never stored on
`GateWidgets`, so `self._widgets.fetch_button` did not exist and the disable
was a silent no-op -- hidden because the suppression tuple included
`AttributeError`. Coverage would have reported 100% on a guard that did
nothing. Guarding on `_fetch_result` needs no widget at all.

Also adds `INTERACTIVE_TIMEOUT_SECONDS = 2.0` for the paths a user waits on.
`SYNC_TIMEOUT_SECONDS` only ever reached GitHub: crdt_sync v0.6.0's
`firebase_client_for` takes no timeout, so Firebase used its 15s default and
one hung session refresh stalled the lock window -- the journal has five such
refresh failures in five days. Verified to fail closed to the local decision.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LJ5YnA3CERitQ57HjMu2Qz
2026-08-21 13:07:37 +02:00

168 lines
6.2 KiB
Python

"""Choosing and building the remote store one sync tick will use.
Split out of ``_sync.py`` for file size.
Two backends, and the tick needs exactly one: Firebase (primary, configured
at ``~/.config/crdt-sync/``) or a GitHub PAT mirror at
``~/.config/diet_guard/sync_token``. Neither configured is not an error --
:func:`_client_for_run` raises :class:`~diet_guard._sync.SyncError` and the
caller logs ``sync not configured`` and no-ops, which is what a machine that
has never been set up should do on a 15-minute timer.
``ConfigError`` subclasses ``Exception`` directly rather than
``RemoteSyncError``, so it is caught explicitly here and translated -- a
sibling-not-subclass relationship that has bitten this file before.
"""
from __future__ import annotations
import logging
from typing import TYPE_CHECKING
from crdt_sync import (
CONFIG_FILE,
ConfigError,
FirebaseAuthError,
GitHubSyncClient,
RemoteSyncError,
firebase_client_for,
mirror_client_for,
)
from diet_guard._constants import (
INTERACTIVE_TIMEOUT_SECONDS,
SYNC_REPO_NAME,
SYNC_REPO_OWNER,
SYNC_TIMEOUT_SECONDS,
SYNC_TOKEN_FILE,
)
from diet_guard._sync_errors import SyncError
if TYPE_CHECKING:
from crdt_sync import RemoteStore
_logger = logging.getLogger(__name__)
def _remote_client(github: GitHubSyncClient) -> RemoteStore:
"""Return the backend to sync against.
Firebase when ``~/.config/crdt-sync/`` is set up, with GitHub kept as a
mirror so a device that has not moved yet still converges; GitHub alone
otherwise. An unconfigured machine keeps syncing exactly as before.
Every sub-sync here (log, budget, food bank, manual bank) takes the client
as a parameter, so this one swap covers all of them.
The config file is checked before constructing anything, so an
unconfigured machine never reaches the network -- otherwise a suite that
blocks real sockets fails here rather than in the code under test.
Rolling back is deleting this function and passing ``github`` straight
through: no data moves either way.
"""
if not CONFIG_FILE.is_file():
return github
try:
return mirror_client_for("diet_guard", github)
except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc:
_logger.warning("Firebase unavailable, syncing via GitHub only: %s", exc)
return github
def _client_for_run() -> RemoteStore:
"""Return the backend for one sync run, requiring only *one* to be set up.
The PAT is no longer mandatory. Firebase is the primary backend now, and
requiring a GitHub token before ever constructing it meant a
Firebase-configured machine with no PAT could not sync at all -- the
commits that fixed exactly this ("Auto-sync on a Firebase-only device")
changed only the Dart side, leaving the Python half behind.
A missing PAT is therefore fatal only when Firebase is *also* unconfigured,
which is the genuine "nothing is set up" case the caller must report.
"""
try:
token = _read_token()
except SyncError:
if not CONFIG_FILE.is_file():
raise
# Firebase-only: there is no GitHub client to mirror to, so use the
# primary directly rather than failing the whole run.
#
# ConfigError is translated rather than propagated: it is *not* a
# RemoteSyncError (it subclasses Exception directly), so letting it out
# would escape every caller's catch tuple and raise a traceback out of
# the gate's fail-closed "Fetch from sync" button -- the exact failure
# the RemoteSyncError swap was made to stop. A config file that exists
# but is unusable is reached by precisely this branch.
try:
return firebase_client_for("diet_guard")
except ConfigError as exc:
message = f"Firebase config at {CONFIG_FILE} is unusable: {exc}"
raise SyncError(message) from exc
return _remote_client(
GitHubSyncClient(
SYNC_REPO_OWNER,
SYNC_REPO_NAME,
token,
timeout_seconds=SYNC_TIMEOUT_SECONDS,
)
)
def _read_token() -> str:
"""Return the saved sync PAT, stripped of trailing whitespace.
Raises:
SyncError: If the token file is missing or empty -- the user has not
completed the one-time github.com setup step yet.
"""
if not SYNC_TOKEN_FILE.exists():
message = (
f"no sync token at {SYNC_TOKEN_FILE} -- create a fine-grained "
"GitHub PAT scoped to the syncs repo's contents and "
f"save it there (mode 600), then re-run sync"
)
raise SyncError(message)
token = SYNC_TOKEN_FILE.read_text().strip()
if not token:
msg = f"{SYNC_TOKEN_FILE} is empty"
raise SyncError(msg)
return token
def _apply_timeout(client: RemoteStore, seconds: float) -> None:
"""Tighten ``client``'s per-request budget, including any mirrored halves.
Reaches for the private ``_timeout_seconds`` because ``crdt_sync`` v0.6.0
exposes no other lever: ``firebase_client_for`` accepts no timeout, so a
Firebase client silently uses its 15s default and one hung request stalls
a path the user is watching. The attribute is read fresh on every request,
so assigning it here is enough.
Set via ``vars()`` rather than ``setattr`` so a client that does not carry
the attribute is left alone instead of gaining a meaningless one. The clean
fix is a ``timeout_seconds`` kwarg upstream; until that ships this keeps the
interactive paths bounded.
"""
for target in (
client,
getattr(client, "primary", None),
getattr(client, "mirror", None),
):
if target is not None and "_timeout_seconds" in vars(target):
vars(target)["_timeout_seconds"] = seconds
def _client_for_interactive_run() -> RemoteStore:
"""Return a sync client budgeted for a path the user is waiting on.
Same backends as :func:`_client_for_run`, but with
:data:`~diet_guard._constants.INTERACTIVE_TIMEOUT_SECONDS` in place of the
background tick's much longer budget.
"""
client = _client_for_run()
_apply_timeout(client, INTERACTIVE_TIMEOUT_SECONDS)
return client