Let a mirror read survive the primary being down

MirrorSyncClient promised that reads consult both backends "so the
cutover order is irrelevant", but every read called self.primary
unguarded. A Firebase outage therefore produced no data at all and the
GitHub mirror was never consulted -- the union read silently degraded to
nothing, precisely when the fallback existed to help.

This is not hypothetical: screen-locker's workout sync hit it on
2026-08-10, where a DNS failure reading the primary took down the whole
locker rather than falling back.

Reads (list_directory, get_file_text, get_string_map) now degrade to the
mirror when the primary raises, and only fail when NEITHER backend can
answer -- returning an empty result would be indistinguishable from "no
data", which callers act on. Writes keep the old asymmetry on purpose: a
write the primary did not accept has not happened, so it must still fail
the tick.

A mirror that cannot bulk-read (GitHub) contributes nothing but is not a
failure, so it cannot by itself make a revision-map read raise. Both
language ports agree on that.

Same fix in the Dart port, which had all three read paths wrong too and
runs on the phone. Two older tests asserted the buggy read-fail-closed
behaviour and are renamed to describe what they actually exercise: the
both-sides-failed case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JS5V44jkHZiH1wC7BH5h5n

crdt-sync/crdt_sync/_mirror.py

@@ -40,6 +40,13 @@ class MirrorSyncClient:
     primary failure fails the tick (fail-closed), while a mirror failure is
     reported and otherwise tolerated. Once the mirror is retired, the old
     backend going away must not break sync.
+
+    **Reads are resilient on BOTH sides; writes are not.** A read is only
+    fail-closed when neither backend can answer. Reads used to call the
+    primary unguarded, so a Firebase outage produced no data at all and the
+    mirror was never consulted -- the union read silently degraded to nothing,
+    contradicting the paragraph above. Writes keep the asymmetry: a write the
+    primary did not accept has not happened, so it must fail the tick.
     """
 
     def __init__(
@@ -63,15 +70,27 @@ class MirrorSyncClient:
         self._on_mirror_failure = on_mirror_failure or _log_mirror_failure
 
     def list_directory(self, path: str) -> list[str]:
-        """Return the union of both backends' entries under ``path``."""
-        names = list(self.primary.list_directory(path))
+        """Return the union of both backends' entries under ``path``.
+
+        A reachable primary that lists nothing is a real answer; a primary
+        that *raises* is not. Reads are the half of this class that exists to
+        be resilient, so a primary failure degrades to the mirror's entries
+        rather than failing the read -- returning nothing because one side is
+        down breaks the union promise exactly when the fallback is most
+        needed. Only a failure on BOTH sides propagates, since then there is
+        genuinely no answer to give.
+        """
+        primary_names = self._try_primary_read(
+            f"list_directory {path}", lambda: self.primary.list_directory(path)
+        )
+        mirror_names = self._try_mirror(
+            f"list_directory {path}", lambda: self.mirror.list_directory(path)
+        )
+        if primary_names is None and mirror_names is None:
+            self._raise_both_failed(f"list_directory {path}")
+        names = list(primary_names or [])
         seen = set(names)
-        for name in (
-            self._try_mirror(
-                f"list_directory {path}", lambda: self.mirror.list_directory(path)
-            )
-            or []
-        ):
+        for name in mirror_names or []:
             if name not in seen:
                 seen.add(name)
                 names.append(name)
@@ -82,8 +101,19 @@ class MirrorSyncClient:
 
         Absent from the primary means "this device has not migrated yet",
         not "no data" -- so it falls through rather than reporting nothing.
+        A primary that *raises* falls through for the same reason (see
+        :meth:`list_directory`); only a failure on both sides propagates.
         """
-        text = self.primary.get_file_text(path)
+        try:
+            text = self.primary.get_file_text(path)
+        except RemoteSyncError as error:
+            self._on_primary_read_failure(f"get_file_text {path}", error)
+            mirror_text = self._try_mirror(
+                f"get_file_text {path}", lambda: self.mirror.get_file_text(path)
+            )
+            if mirror_text is None:
+                self._raise_both_failed(f"get_file_text {path}")
+            return mirror_text
         if text is not None:
             return text
         return self._try_mirror(
@@ -128,17 +158,32 @@ class MirrorSyncClient:
         A device that has not migrated publishes revisions only to the
         mirror; without this it would look revision-less and be
         re-downloaded every tick for the whole trial period.
+
+        Like the other reads, a primary failure degrades to the mirror rather
+        than failing the tick: revisions are a cache, and losing them re-reads
+        data rather than losing it.
         """
         merged: dict[str, str] = {}
         mirror_read = getattr(self.mirror, "get_string_map", None)
+        # A mirror that CANNOT do bulk reads contributes nothing, but that is
+        # not a failure -- it has no revisions to give, which is a real answer.
+        # Only a mirror that tried and threw leaves the read with no answer,
+        # and only then can a failing primary make this raise.
+        mirror_answered = mirror_read is None
         if mirror_read is not None:
-            merged.update(
-                self._try_mirror(f"get_string_map {path}", lambda: mirror_read(path))
-                or {}
+            mirror_map = self._try_mirror(
+                f"get_string_map {path}", lambda: mirror_read(path)
             )
+            mirror_answered = mirror_map is not None
+            merged.update(mirror_map or {})
         primary_read = getattr(self.primary, "get_string_map", None)
         if primary_read is not None:
-            merged.update(primary_read(path))
+            primary_map = self._try_primary_read(
+                f"get_string_map {path}", lambda: primary_read(path)
+            )
+            if primary_map is None and not mirror_answered:
+                self._raise_both_failed(f"get_string_map {path}")
+            merged.update(primary_map or {})
         return merged
 
     def _try_mirror(self, operation: str, run: Callable[[], _T]) -> _T | None:
@@ -149,6 +194,36 @@ class MirrorSyncClient:
             self._on_mirror_failure(operation, error)
             return None
 
+    def _try_primary_read(self, operation: str, run: Callable[[], _T]) -> _T | None:
+        """Run a primary READ, reporting rather than propagating failure.
+
+        Reads only -- writes stay fail-closed, since a write the primary did
+        not accept has not happened and must fail the tick.
+        """
+        try:
+            return run()
+        except RemoteSyncError as error:
+            self._on_primary_read_failure(operation, error)
+            return None
+
+    def _on_primary_read_failure(self, operation: str, error: Exception) -> None:
+        """Warn that a read fell back to the mirror."""
+        _logger.warning(
+            "Primary backend failed during %s: %s — falling back to the mirror",
+            operation,
+            error,
+        )
+
+    def _raise_both_failed(self, operation: str) -> None:
+        """Raise when neither backend could answer a read.
+
+        Fail closed rather than returning an empty result: an empty read is
+        indistinguishable from "no data", and callers act on that by treating
+        remote state as absent.
+        """
+        msg = f"both primary and mirror backends failed during {operation}"
+        raise RemoteSyncError(msg)
+
 
 def _log_mirror_failure(operation: str, error: Exception) -> None:
     """Warn loudly, so a rotting fallback is visible before it is needed."""

crdt-sync/crdt_sync/tests/test_mirror.py

@@ -203,7 +203,13 @@ class TestReads:
         assert client.get_file_text("ns/pc/log.json") is None
         assert failures == ["get_file_text ns/pc/log.json"]
 
-    def test_a_primary_read_failure_fails_the_tick(self) -> None:
+    def test_a_primary_read_failure_with_nothing_in_the_mirror_raises(
+        self,
+    ) -> None:
+        # Named for what it actually exercises: the primary raises AND the
+        # mirror does not hold the file, so neither side has an answer. A
+        # primary failure alone no longer fails the read -- see
+        # TestPrimaryReadFallback.
         client, _, _, _ = _mirror(primary_failing=True)
         with pytest.raises(RemoteSyncError):
             client.get_file_text("ns/pc/log.json")
@@ -231,12 +237,82 @@ class TestListDirectory:
         assert client.list_directory("ns") == ["pc"]
         assert failures == ["list_directory ns"]
 
-    def test_a_primary_failure_fails_the_tick(self) -> None:
-        client, _, _, _ = _mirror(primary_failing=True)
+    def test_a_primary_failure_degrades_to_the_mirror_list(self) -> None:
+        # READS are resilient on both sides: a Firebase outage must not hide
+        # the mirror's devices. This used to raise, which made a primary
+        # outage look like "no devices exist" -- the union read silently
+        # degrading to nothing, precisely when the fallback was needed.
+        client, _, _, _ = _mirror(
+            primary_failing=True, mirror_files={"ns/phone/log.json": "{}"}
+        )
+        assert client.list_directory("ns") == ["phone"]
+
+    def test_both_backends_failing_raises(self) -> None:
+        # With no answer from either side, fail closed: an empty list is
+        # indistinguishable from "no devices", and callers act on that.
+        client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True)
         with pytest.raises(RemoteSyncError):
             client.list_directory("ns")
 
 
+class TestPrimaryReadFallback:
+    """A failing primary must never hide data the mirror can still serve."""
+
+    def test_get_file_text_falls_back_to_the_mirror(self) -> None:
+        client, _, _, _ = _mirror(
+            primary_failing=True, mirror_files={"ns/phone/log.json": "{}"}
+        )
+        assert client.get_file_text("ns/phone/log.json") == "{}"
+
+    def test_get_file_text_raises_when_both_fail(self) -> None:
+        client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True)
+        with pytest.raises(RemoteSyncError):
+            client.get_file_text("ns/phone/log.json")
+
+    def test_get_file_text_raises_when_primary_fails_and_mirror_lacks_it(
+        self,
+    ) -> None:
+        # A reachable mirror that simply does not hold the file is not an
+        # answer either, since the primary's copy was never read.
+        client, _, _, _ = _mirror(primary_failing=True)
+        with pytest.raises(RemoteSyncError):
+            client.get_file_text("ns/phone/log.json")
+
+    def test_revision_map_falls_back_to_the_mirror(self) -> None:
+        client, _, _, _ = _mirror(
+            primary_failing=True, mirror_files={"ns/revs/phone": "r1"}
+        )
+        assert client.get_string_map("ns/revs") == {"phone": "r1"}
+
+    def test_revision_map_raises_when_both_fail(self) -> None:
+        client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True)
+        with pytest.raises(RemoteSyncError):
+            client.get_string_map("ns/revs")
+
+    def test_revision_map_tolerates_a_mirror_that_cannot_bulk_read(
+        self,
+    ) -> None:
+        # A mirror with no bulk-read capability (GitHub) contributes nothing,
+        # but that is not a failure -- it simply has no revisions to add, so a
+        # working primary's map must still come back. Matches the Dart side,
+        # where the capability check runs inside the guarded closure.
+        client = MirrorSyncClient(
+            FakeStore({"ns/revs/pc": "r1"}),
+            FakeStoreWithoutBulkRead(),
+            on_mirror_failure=lambda *_: None,
+        )
+        assert client.get_string_map("ns/revs") == {"pc": "r1"}
+
+    def test_writes_stay_fail_closed_on_a_primary_failure(self) -> None:
+        # The read fix must NOT loosen writes: an unaccepted write has not
+        # happened, so it must still fail the tick.
+        client, _, _, _ = _mirror(primary_failing=True)
+        with pytest.raises(RemoteSyncError):
+            client.put_file_text("ns/pc/log.json", "{}", message="m")
+        with pytest.raises(RemoteSyncError):
+            client.delete_file("ns/pc/log.json")
+
+
 class TestRevisionMaps:
     def test_merge_both_backends_with_the_primary_winning(self) -> None:
         client, _, _, _ = _mirror(

crdt-sync/pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "crdt-sync"
-version = "0.6.0"
+version = "0.7.0"
 description = "Shared CRDT merge scheme + GitHub-Contents-API sync transport for cross-device apps"
 requires-python = ">=3.10"
 dependencies = ["requests"]

crdt_sync_dart/lib/src/mirror_store.dart

@@ -8,8 +8,7 @@ library;
 import 'remote_store.dart';
 
 /// Called when the mirror backend fails, so the failure is loud but survivable.
-typedef MirrorFailureHandler =
-    void Function(String operation, Object error);
+typedef MirrorFailureHandler = void Function(String operation, Object error);
 
 /// Writes to both backends; reads from both and prefers the primary.
 ///
@@ -24,11 +23,19 @@ typedef MirrorFailureHandler =
 /// primary failure fails the tick (fail-closed), while a mirror failure is
 /// reported through [onMirrorFailure] and otherwise tolerated. Once the
 /// mirror is retired, the old backend going away must not break sync.
+///
+/// **Reads are resilient on BOTH sides; writes are not.** A read is only
+/// fail-closed when neither backend can answer. Reads used to call the
+/// primary unguarded, so a Firebase outage produced no data at all and the
+/// mirror was never consulted -- the union read silently degraded to nothing,
+/// contradicting the paragraph above. Writes keep the asymmetry: a write the
+/// primary did not accept has not happened, so it must fail the tick.
 class MirrorStore implements RemoteStore, BulkMapReader {
   MirrorStore({
     required this.primary,
     required this.mirror,
     this.onMirrorFailure,
+    this.onPrimaryReadFailure,
   });
 
   /// The authoritative backend. Its failures fail the tick.
@@ -41,25 +48,49 @@ class MirrorStore implements RemoteStore, BulkMapReader {
   /// the fallback rot unnoticed until the day it was needed.
   final MirrorFailureHandler? onMirrorFailure;
 
+  /// Notified when a primary READ fails and the mirror is used instead.
+  ///
+  /// Separate from [onMirrorFailure] so "the backend we are migrating TO is
+  /// down" is distinguishable from "the backend we are retiring is down".
+  final MirrorFailureHandler? onPrimaryReadFailure;
+
   @override
   Future<List<String>> listDirectory(String path) async {
-    final names = <String>{...await primary.listDirectory(path)};
-    await _tryMirror('listDirectory $path', () async {
+    // A reachable primary that lists nothing is a real answer; a primary that
+    // THROWS is not. Reads are the half of this class that exists to be
+    // resilient, so a primary failure degrades to the mirror's entries rather
+    // than failing the read -- returning nothing because one side is down
+    // breaks the union promise exactly when the fallback is most needed.
+    final names = <String>{};
+    final primaryOk = await _tryPrimaryRead('listDirectory $path', () async {
+      names.addAll(await primary.listDirectory(path));
+    });
+    final mirrorOk = await _tryMirror('listDirectory $path', () async {
       names.addAll(await mirror.listDirectory(path));
     });
+    if (!primaryOk && !mirrorOk) {
+      _raiseBothFailed('listDirectory $path');
+    }
     return names.toList();
   }
 
   @override
   Future<String?> getFileText(String path) async {
-    final text = await primary.getFileText(path);
-    if (text != null) return text;
+    String? text;
+    final primaryOk = await _tryPrimaryRead('getFileText $path', () async {
+      text = await primary.getFileText(path);
+    });
+    if (primaryOk && text != null) return text;
     // Absent from the primary means "this device has not migrated yet", not
-    // "no data" -- so fall through rather than reporting nothing.
+    // "no data" -- so fall through rather than reporting nothing. A primary
+    // that threw falls through for the same reason.
     String? fallback;
-    await _tryMirror('getFileText $path', () async {
+    final mirrorOk = await _tryMirror('getFileText $path', () async {
       fallback = await mirror.getFileText(path);
     });
+    if (!primaryOk && (!mirrorOk || fallback == null)) {
+      _raiseBothFailed('getFileText $path');
+    }
     return fallback;
   }
 
@@ -104,14 +135,21 @@ class MirrorStore implements RemoteStore, BulkMapReader {
     // Pattern binds rather than `is` promotion: BulkMapReader is not a
     // subtype of RemoteStore, and Dart only promotes to subtypes of the
     // declared type, so the intersection has to be named explicitly.
-    await _tryMirror('getStringMap $path', () async {
+    final mirrorOk = await _tryMirror('getStringMap $path', () async {
       if (mirror case final BulkMapReader reader) {
         merged.addAll(await reader.getStringMap(path));
       }
     });
-    // Primary applied last so its entries win on conflict.
-    if (primary case final BulkMapReader reader) {
-      merged.addAll(await reader.getStringMap(path));
+    // Primary applied last so its entries win on conflict. Like the other
+    // reads, a primary failure degrades to the mirror: revisions are a cache,
+    // and losing them re-reads data rather than losing it.
+    final primaryOk = await _tryPrimaryRead('getStringMap $path', () async {
+      if (primary case final BulkMapReader reader) {
+        merged.addAll(await reader.getStringMap(path));
+      }
+    });
+    if (!primaryOk && !mirrorOk) {
+      _raiseBothFailed('getStringMap $path');
     }
     return merged;
   }
@@ -123,11 +161,43 @@ class MirrorStore implements RemoteStore, BulkMapReader {
   }
 
   /// Runs a mirror operation, reporting rather than propagating its failure.
-  Future<void> _tryMirror(String operation, Future<void> Function() run) async {
+  ///
+  /// Returns whether it succeeded, so a read can tell "the mirror had nothing"
+  /// from "the mirror could not be reached".
+  Future<bool> _tryMirror(String operation, Future<void> Function() run) async {
     try {
       await run();
+      return true;
     } on RemoteSyncError catch (error) {
       onMirrorFailure?.call(operation, error);
+      return false;
     }
   }
+
+  /// Runs a primary READ, reporting rather than propagating its failure.
+  ///
+  /// Reads only -- writes stay fail-closed, since a write the primary did not
+  /// accept has not happened and must fail the tick.
+  Future<bool> _tryPrimaryRead(
+    String operation,
+    Future<void> Function() run,
+  ) async {
+    try {
+      await run();
+      return true;
+    } on RemoteSyncError catch (error) {
+      onPrimaryReadFailure?.call(operation, error);
+      return false;
+    }
+  }
+
+  /// Fails closed when neither backend could answer a read.
+  ///
+  /// An empty result is indistinguishable from "no data", and callers act on
+  /// that by treating remote state as absent.
+  Never _raiseBothFailed(String operation) {
+    throw RemoteSyncError(
+      'both primary and mirror backends failed during $operation',
+    );
+  }
 }

crdt_sync_dart/pubspec.yaml

@@ -2,7 +2,7 @@ name: crdt_sync
 description: >-
   Shared CRDT merge scheme + GitHub-Contents-API sync transport for
   cross-device apps. Pure Dart, no Flutter dependency.
-version: 0.8.0
+version: 0.9.0
 publish_to: 'none'
 
 environment:

crdt_sync_dart/test/mirror_store_test.dart

@@ -82,8 +82,7 @@ class _FakeStoreWithoutBulkRead implements RemoteStore {
   final _FakeStore _inner;
 
   @override
-  Future<List<String>> listDirectory(String path) =>
-      _inner.listDirectory(path);
+  Future<List<String>> listDirectory(String path) => _inner.listDirectory(path);
   @override
   Future<String?> getFileText(String path) => _inner.getFileText(path);
   @override
@@ -98,7 +97,12 @@ class _FakeStoreWithoutBulkRead implements RemoteStore {
   void close() => _inner.close();
 }
 
-({MirrorStore store, _FakeStore primary, _FakeStore mirror, List<String> failures})
+({
+  MirrorStore store,
+  _FakeStore primary,
+  _FakeStore mirror,
+  List<String> failures,
+})
 _mirror({
   Map<String, String>? primaryFiles,
   Map<String, String>? mirrorFiles,
@@ -194,7 +198,11 @@ void main() {
       expect(m.failures, ['getFileText ns/pc/log.json']);
     });
 
-    test('a primary read failure fails the tick', () async {
+    test('a primary read failure with nothing in the mirror throws', () async {
+      // Named for what it actually exercises: the primary throws AND the
+      // mirror does not hold the file, so neither side has an answer. A
+      // primary failure alone no longer fails the read -- see the
+      // "primary read fallback" group.
       final m = _mirror(primaryFailing: true);
       await expectLater(
         () => m.store.getFileText('ns/pc/log.json'),
@@ -229,8 +237,22 @@ void main() {
       expect(m.failures, ['listDirectory ns']);
     });
 
-    test('a primary failure fails the tick', () async {
-      final m = _mirror(primaryFailing: true);
+    test('a primary failure degrades to the mirror list', () async {
+      // READS are resilient on both sides: a Firebase outage must not hide
+      // the mirror's devices. This used to throw, which made a primary outage
+      // look like "no devices exist" -- the union read silently degrading to
+      // nothing, precisely when the fallback was needed.
+      final m = _mirror(
+        primaryFailing: true,
+        mirrorFiles: {'ns/phone/log.json': '{}'},
+      );
+      expect(await m.store.listDirectory('ns'), ['phone']);
+    });
+
+    test('both backends failing throws', () async {
+      // With no answer from either side, fail closed: an empty list is
+      // indistinguishable from "no devices", and callers act on that.
+      final m = _mirror(primaryFailing: true, mirrorFailing: true);
       await expectLater(
         () => m.store.listDirectory('ns'),
         throwsA(isA<RemoteSyncError>()),
@@ -238,6 +260,75 @@ void main() {
     });
   });
 
+  group('primary read fallback', () {
+    test('getFileText falls back to the mirror', () async {
+      final m = _mirror(
+        primaryFailing: true,
+        mirrorFiles: {'ns/phone/log.json': '{}'},
+      );
+      expect(await m.store.getFileText('ns/phone/log.json'), '{}');
+    });
+
+    test('getFileText throws when both fail', () async {
+      final m = _mirror(primaryFailing: true, mirrorFailing: true);
+      await expectLater(
+        () => m.store.getFileText('ns/phone/log.json'),
+        throwsA(isA<RemoteSyncError>()),
+      );
+    });
+
+    test('getFileText throws when primary fails and mirror lacks it', () async {
+      // A reachable mirror that simply does not hold the file is not an
+      // answer either, since the primary's copy was never read.
+      final m = _mirror(primaryFailing: true);
+      await expectLater(
+        () => m.store.getFileText('ns/phone/log.json'),
+        throwsA(isA<RemoteSyncError>()),
+      );
+    });
+
+    test('getStringMap falls back to the mirror', () async {
+      final m = _mirror(
+        primaryFailing: true,
+        mirrorFiles: {'ns/revs/phone': 'r1'},
+      );
+      expect(await m.store.getStringMap('ns/revs'), {'phone': 'r1'});
+    });
+
+    test('getStringMap throws when both fail', () async {
+      final m = _mirror(primaryFailing: true, mirrorFailing: true);
+      await expectLater(
+        () => m.store.getStringMap('ns/revs'),
+        throwsA(isA<RemoteSyncError>()),
+      );
+    });
+
+    test('getStringMap tolerates a mirror that cannot bulk-read', () async {
+      // A mirror with no bulk-read capability (GitHub) contributes nothing,
+      // but that is not a failure -- it simply has no revisions to add, so a
+      // working primary's map must still come back.
+      final store = MirrorStore(
+        primary: _FakeStore(files: {'ns/revs/pc': 'r1'}),
+        mirror: _FakeStoreWithoutBulkRead(),
+      );
+      expect(await store.getStringMap('ns/revs'), {'pc': 'r1'});
+    });
+
+    test('writes stay fail-closed on a primary failure', () async {
+      // The read fix must NOT loosen writes: an unaccepted write has not
+      // happened, so it must still fail the tick.
+      final m = _mirror(primaryFailing: true);
+      await expectLater(
+        () => m.store.putFileText('ns/pc/log.json', '{}', message: 'm'),
+        throwsA(isA<RemoteSyncError>()),
+      );
+      await expectLater(
+        () => m.store.deleteFile('ns/pc/log.json'),
+        throwsA(isA<RemoteSyncError>()),
+      );
+    });
+  });
+
   group('revision maps', () {
     test('merge both backends, primary winning on conflict', () async {
       final m = _mirror(