mirror of
https://github.com/kuhyx/todo-app.git
synced 2026-11-14 20:50:54 +01:00
Surface sync outcomes so devices can't silently diverge
The desktop and phone split-brained for 5 days (39 vs 30 notes) after the crdt_sync cutover because one device kept running the pre-migration build — and nothing in the UI ever said a sync had failed or skipped a peer: - SyncResult now reports peer files that were listed but couldn't be fetched/decoded (skippedFiles) instead of dropping them silently. - Auto-sync no longer swallows errors: every sync (manual or automatic) records a persisted last-sync status shown in a status line under the editor, with failures in the error color. - Desktop actually background-syncs now: focus loss (inactive/hidden, debounced 5s) triggers a sync — Flutter Linux rarely delivers the `paused` event the old code relied on. `paused` still syncs immediately for Android. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K5C5jZ2c4mTaGedK7C1dDv
This commit is contained in:
parent
80e86157ee
commit
7564a6569f
@ -4,7 +4,11 @@ import '../data/note_repository.dart';
|
||||
|
||||
/// Outcome of a sync run, for surfacing in the UI.
|
||||
class SyncResult {
|
||||
const SyncResult({required this.mergedDevices, required this.pushed});
|
||||
const SyncResult({
|
||||
required this.mergedDevices,
|
||||
required this.pushed,
|
||||
this.skippedFiles = const [],
|
||||
});
|
||||
|
||||
/// How many other devices' logs were pulled and merged.
|
||||
final int mergedDevices;
|
||||
@ -12,9 +16,15 @@ class SyncResult {
|
||||
/// Whether this device pushed its own log.
|
||||
final bool pushed;
|
||||
|
||||
/// Peer files that were listed but could not be fetched or decoded
|
||||
/// (vanished, corrupt, or foreign format) and were left out of the merge.
|
||||
/// Surfaced so a device silently dropping out of sync is visible.
|
||||
final List<String> skippedFiles;
|
||||
|
||||
@override
|
||||
String toString() =>
|
||||
'SyncResult(mergedDevices: $mergedDevices, pushed: $pushed)';
|
||||
'SyncResult(mergedDevices: $mergedDevices, pushed: $pushed, '
|
||||
'skippedFiles: $skippedFiles)';
|
||||
}
|
||||
|
||||
/// Synchronises a [NoteRepository] with a GitHub repo used as dumb storage.
|
||||
@ -43,12 +53,18 @@ class SyncService {
|
||||
// 1. Pull: list all device log files, merge everyone else's.
|
||||
final names = await github.listDirectory(notesDir);
|
||||
var merged = 0;
|
||||
final skipped = <String>[];
|
||||
for (final name in names) {
|
||||
if (name == ownFileName) continue; // our own file; skip it.
|
||||
final text = await github.getFileText('$notesDir/$name');
|
||||
if (text == null) continue;
|
||||
final remote = _decodeLog(text);
|
||||
if (remote == null) continue; // Skip a corrupt/foreign file.
|
||||
final remote = text == null ? null : _decodeLog(text);
|
||||
if (remote == null) {
|
||||
// A vanished or undecodable peer file must not abort the sync, but
|
||||
// it must not vanish silently either — that's how a device drops
|
||||
// out of sync unnoticed. Report it in the result instead.
|
||||
skipped.add(name);
|
||||
continue;
|
||||
}
|
||||
await repo.importLog(remote);
|
||||
merged++;
|
||||
}
|
||||
@ -61,7 +77,11 @@ class SyncService {
|
||||
message: 'sync: $nodeId @ ${DateTime.now().toUtc().toIso8601String()}',
|
||||
);
|
||||
|
||||
return SyncResult(mergedDevices: merged, pushed: true);
|
||||
return SyncResult(
|
||||
mergedDevices: merged,
|
||||
pushed: true,
|
||||
skippedFiles: skipped,
|
||||
);
|
||||
}
|
||||
|
||||
/// Parses a remote note log, returning `null` for a corrupt or wrong-shape
|
||||
|
||||
@ -4,6 +4,7 @@ import 'dart:io';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:path_provider/path_provider.dart';
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
import 'package:uuid/uuid.dart';
|
||||
|
||||
import '../data/note.dart';
|
||||
@ -43,6 +44,10 @@ class CaptureScreen extends StatefulWidget {
|
||||
/// file-backed instance is created); tests pass a fake with in-memory IO.
|
||||
final LocalBackup? localBackup;
|
||||
|
||||
/// How long focus must stay lost before a background sync fires. Public so
|
||||
/// tests can pump exactly past it.
|
||||
static const autoSyncDebounce = Duration(seconds: 5);
|
||||
|
||||
@override
|
||||
State<CaptureScreen> createState() => _CaptureScreenState();
|
||||
}
|
||||
@ -85,6 +90,19 @@ class _CaptureScreenState extends State<CaptureScreen>
|
||||
SyncSettings? _settings;
|
||||
bool _syncing = false;
|
||||
|
||||
/// Outcome of the most recent sync attempt (manual or automatic), driving
|
||||
/// the tiny status line under the editor. Persisted so a failure that
|
||||
/// happened just before the app closed is still visible on next launch.
|
||||
final ValueNotifier<SyncStatus?> _lastSync = ValueNotifier<SyncStatus?>(null);
|
||||
|
||||
/// Debounces focus-loss sync triggers so desktop alt-tab flicker doesn't
|
||||
/// hammer GitHub with a request per focus change.
|
||||
Timer? _autoSyncDebounce;
|
||||
|
||||
static const _kLastSyncTime = 'sync.lastTime';
|
||||
static const _kLastSyncOk = 'sync.lastOk';
|
||||
static const _kLastSyncDetail = 'sync.lastDetail';
|
||||
|
||||
/// Hides the Priority/Status row while the editor's own bare-guided chrome
|
||||
/// (template/mode selectors) is also hidden, so the two stay in lockstep.
|
||||
bool _chromeVisible = true;
|
||||
@ -106,6 +124,7 @@ class _CaptureScreenState extends State<CaptureScreen>
|
||||
// fires on writes), so a deleted/stale BACKLOG.md is regenerated even if
|
||||
// the user makes no edits this session.
|
||||
_localBackup.schedule();
|
||||
_restoreSyncStatus();
|
||||
SyncSettings.load().then((s) {
|
||||
if (!mounted) return;
|
||||
setState(() => _settings = s);
|
||||
@ -116,12 +135,49 @@ class _CaptureScreenState extends State<CaptureScreen>
|
||||
@override
|
||||
void dispose() {
|
||||
WidgetsBinding.instance.removeObserver(this);
|
||||
_autoSyncDebounce?.cancel();
|
||||
_changesSub?.cancel();
|
||||
_localBackup.dispose();
|
||||
_lastSavedAt.dispose();
|
||||
_lastSync.dispose();
|
||||
super.dispose();
|
||||
}
|
||||
|
||||
/// Reloads the persisted last-sync outcome so the status line survives a
|
||||
/// restart (a failure just before closing must not disappear).
|
||||
Future<void> _restoreSyncStatus() async {
|
||||
final prefs = await SharedPreferences.getInstance();
|
||||
final time = DateTime.tryParse(prefs.getString(_kLastSyncTime) ?? '');
|
||||
if (!mounted || time == null || _lastSync.value != null) return;
|
||||
_lastSync.value = SyncStatus(
|
||||
time: time,
|
||||
ok: prefs.getBool(_kLastSyncOk) ?? true,
|
||||
detail: prefs.getString(_kLastSyncDetail) ?? '',
|
||||
);
|
||||
}
|
||||
|
||||
/// Records a sync outcome in the UI notifier and persists it.
|
||||
Future<void> _recordSyncStatus({
|
||||
required bool ok,
|
||||
required String detail,
|
||||
}) async {
|
||||
final status = SyncStatus(time: DateTime.now(), ok: ok, detail: detail);
|
||||
if (mounted) _lastSync.value = status;
|
||||
final prefs = await SharedPreferences.getInstance();
|
||||
await prefs.setString(_kLastSyncTime, status.time.toIso8601String());
|
||||
await prefs.setBool(_kLastSyncOk, ok);
|
||||
await prefs.setString(_kLastSyncDetail, detail);
|
||||
}
|
||||
|
||||
/// Human-readable one-liner for a sync outcome, shared by the snackbar and
|
||||
/// the status line. Skipped files are called out so a peer device silently
|
||||
/// dropping out of the merge is visible.
|
||||
static String _describe(SyncResult result) {
|
||||
final skipped = result.skippedFiles.length;
|
||||
return 'merged ${result.mergedDevices} device(s)'
|
||||
'${skipped == 0 ? '' : ', skipped $skipped unreadable file(s)'}';
|
||||
}
|
||||
|
||||
/// Restores notes from the local backup file, but only into an empty DB so a
|
||||
/// stale backup never clobbers existing notes (merge-by-id stays safe too).
|
||||
Future<void> _recoverFromBackup() async {
|
||||
@ -161,12 +217,23 @@ class _CaptureScreenState extends State<CaptureScreen>
|
||||
@override
|
||||
void didChangeAppLifecycleState(AppLifecycleState state) {
|
||||
// Push on background so the remote (the durable store) stays near-current.
|
||||
if (state == AppLifecycleState.paused) _autoSync();
|
||||
if (state == AppLifecycleState.paused) {
|
||||
// Immediate: Android may kill the app before a debounce timer fires.
|
||||
_autoSyncDebounce?.cancel();
|
||||
_autoSync();
|
||||
} else if (state == AppLifecycleState.inactive ||
|
||||
state == AppLifecycleState.hidden) {
|
||||
// Focus loss — the only background signal Flutter Linux delivers
|
||||
// reliably (`paused` rarely arrives on desktop). Debounced so alt-tab
|
||||
// flicker doesn't fire a request per focus change.
|
||||
_autoSyncDebounce?.cancel();
|
||||
_autoSyncDebounce = Timer(CaptureScreen.autoSyncDebounce, _autoSync);
|
||||
}
|
||||
}
|
||||
|
||||
/// Best-effort background sync: silent, skips when unconfigured, and never
|
||||
/// overlaps itself. Failures are swallowed — the manual Sync button is the
|
||||
/// place that surfaces errors.
|
||||
/// Best-effort background sync: no snackbar, skips when unconfigured, and
|
||||
/// never overlaps itself. Failures are not swallowed silently — the outcome
|
||||
/// lands in the status line so drift can't go unnoticed for days.
|
||||
Future<void> _autoSync() async {
|
||||
final settings = _settings;
|
||||
if (_autoSyncing || settings == null || !settings.isConfigured) return;
|
||||
@ -178,9 +245,12 @@ class _CaptureScreenState extends State<CaptureScreen>
|
||||
httpClient: widget.httpClient,
|
||||
);
|
||||
try {
|
||||
await _syncService.sync(widget.repository, client);
|
||||
} catch (_) {
|
||||
// Best-effort: ignore (offline, transient GitHub errors, etc.).
|
||||
final result = await _syncService.sync(widget.repository, client);
|
||||
await _recordSyncStatus(ok: true, detail: _describe(result));
|
||||
} catch (e) {
|
||||
// Offline or a transient GitHub error: still no snackbar (this path
|
||||
// must never interrupt capture), but the status line shows it.
|
||||
await _recordSyncStatus(ok: false, detail: '$e');
|
||||
} finally {
|
||||
client.close();
|
||||
_autoSyncing = false;
|
||||
@ -233,8 +303,11 @@ class _CaptureScreenState extends State<CaptureScreen>
|
||||
);
|
||||
try {
|
||||
final result = await _syncService.sync(widget.repository, client);
|
||||
_showSnack('Synced: merged ${result.mergedDevices} device(s)');
|
||||
final detail = _describe(result);
|
||||
await _recordSyncStatus(ok: true, detail: detail);
|
||||
_showSnack('Synced: $detail');
|
||||
} catch (e) {
|
||||
await _recordSyncStatus(ok: false, detail: '$e');
|
||||
_showSnack('Sync failed: $e');
|
||||
} finally {
|
||||
client.close();
|
||||
@ -419,17 +492,37 @@ class _CaptureScreenState extends State<CaptureScreen>
|
||||
),
|
||||
),
|
||||
const SizedBox(height: 8),
|
||||
// Leave room so the Save FAB doesn't cover the save indicator.
|
||||
// Leave room so the Save FAB doesn't cover the indicators.
|
||||
Padding(
|
||||
padding: const EdgeInsets.only(right: 96),
|
||||
child: ValueListenableBuilder<DateTime?>(
|
||||
valueListenable: _lastSavedAt,
|
||||
builder: (context, savedAt, _) => Text(
|
||||
savedAt == null
|
||||
? 'Autosaves as you type'
|
||||
: 'Saved locally at ${_formatTime(savedAt)}',
|
||||
style: theme.textTheme.bodySmall,
|
||||
),
|
||||
child: Column(
|
||||
crossAxisAlignment: CrossAxisAlignment.start,
|
||||
children: [
|
||||
ValueListenableBuilder<DateTime?>(
|
||||
valueListenable: _lastSavedAt,
|
||||
builder: (context, savedAt, _) => Text(
|
||||
savedAt == null
|
||||
? 'Autosaves as you type'
|
||||
: 'Saved locally at ${_formatTime(savedAt)}',
|
||||
style: theme.textTheme.bodySmall,
|
||||
),
|
||||
),
|
||||
ValueListenableBuilder<SyncStatus?>(
|
||||
valueListenable: _lastSync,
|
||||
builder: (context, status, _) {
|
||||
if (status == null) return const SizedBox.shrink();
|
||||
return Text(
|
||||
'${status.ok ? 'Synced' : 'Sync failed'} at '
|
||||
'${_formatTime(status.time)} · ${status.detail}',
|
||||
style: theme.textTheme.bodySmall?.copyWith(
|
||||
color: status.ok ? null : theme.colorScheme.error,
|
||||
),
|
||||
maxLines: 1,
|
||||
overflow: TextOverflow.ellipsis,
|
||||
);
|
||||
},
|
||||
),
|
||||
],
|
||||
),
|
||||
),
|
||||
],
|
||||
@ -452,6 +545,24 @@ class _CaptureScreenState extends State<CaptureScreen>
|
||||
}
|
||||
}
|
||||
|
||||
/// Outcome of a sync attempt, shown in the capture screen's status line.
|
||||
class SyncStatus {
|
||||
const SyncStatus({
|
||||
required this.time,
|
||||
required this.ok,
|
||||
required this.detail,
|
||||
});
|
||||
|
||||
/// When the attempt finished.
|
||||
final DateTime time;
|
||||
|
||||
/// Whether the sync succeeded.
|
||||
final bool ok;
|
||||
|
||||
/// One-line summary: merged/skipped counts on success, the error otherwise.
|
||||
final String detail;
|
||||
}
|
||||
|
||||
/// A compact labelled dropdown for picking an enum value (priority/status).
|
||||
///
|
||||
/// Generic over the enum type [T] so the same control drives both pickers
|
||||
|
||||
@ -1,3 +1,5 @@
|
||||
import 'dart:convert';
|
||||
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
@ -234,7 +236,40 @@ void main() {
|
||||
await tester.pump();
|
||||
await tester.pump();
|
||||
|
||||
expect(find.textContaining('Sync failed'), findsOneWidget);
|
||||
// 'Sync failed:' is the snackbar; the status line says 'Sync failed at'.
|
||||
expect(find.textContaining('Sync failed:'), findsOneWidget);
|
||||
expect(find.textContaining('Sync failed at'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('manual sync reports peer files it could not decode', (
|
||||
tester,
|
||||
) async {
|
||||
// One peer file exists but holds garbage → merged 0, skipped 1.
|
||||
final mock = MockClient((req) async {
|
||||
if (req.method == 'PUT') return http.Response('{}', 200);
|
||||
if (!req.url.path.contains('/contents/')) return http.Response('{}', 200);
|
||||
if (req.url.path.endsWith('/todo-sync/notes')) {
|
||||
return http.Response('[{"name": "bad.json"}]', 200);
|
||||
}
|
||||
if (req.url.path.endsWith('/bad.json')) {
|
||||
return http.Response(
|
||||
'{"content": "${base64Encode(utf8.encode('{not a log'))}"}',
|
||||
200,
|
||||
);
|
||||
}
|
||||
return http.Response('', 404); // own-file sha probe → create fresh
|
||||
});
|
||||
await pumpCapture(tester, prefs: configuredPrefs, httpClient: mock);
|
||||
|
||||
await tester.tap(find.byTooltip('Sync'));
|
||||
await tester.pump();
|
||||
await tester.pump();
|
||||
await tester.pump();
|
||||
|
||||
expect(
|
||||
find.textContaining('skipped 1 unreadable file(s)'),
|
||||
findsWidgets, // snackbar and status line both call it out
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('returning from settings adopts the saved configuration', (
|
||||
@ -311,14 +346,79 @@ void main() {
|
||||
expect(methods, contains('PUT'));
|
||||
});
|
||||
|
||||
testWidgets('auto-sync failure is silent (no snackbar)', (tester) async {
|
||||
testWidgets('auto-sync failure shows no snackbar but flags the status line', (
|
||||
tester,
|
||||
) async {
|
||||
final mock = MockClient((_) async => throw Exception('offline'));
|
||||
await pumpCapture(tester, prefs: configuredPrefs, httpClient: mock);
|
||||
await tester.pump();
|
||||
await tester.pump();
|
||||
|
||||
expect(find.textContaining('Sync failed'), findsNothing);
|
||||
expect(find.textContaining('Synced'), findsNothing);
|
||||
// Capture must never be interrupted by a snackbar…
|
||||
expect(find.byType(SnackBar), findsNothing);
|
||||
// …but the failure is no longer swallowed: the status line shows it.
|
||||
expect(find.textContaining('Sync failed at'), findsOneWidget);
|
||||
expect(find.textContaining('offline'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('auto-sync success shows when and what was merged', (
|
||||
tester,
|
||||
) async {
|
||||
final methods = <String>[];
|
||||
await pumpCapture(
|
||||
tester,
|
||||
prefs: configuredPrefs,
|
||||
httpClient: recordingMock(methods),
|
||||
);
|
||||
await tester.pump();
|
||||
await tester.pump();
|
||||
|
||||
expect(find.textContaining('Synced at'), findsOneWidget);
|
||||
expect(find.textContaining('merged 0 device(s)'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('restores the last sync outcome on launch', (tester) async {
|
||||
// Unconfigured (no token) so no launch auto-sync overwrites the restored
|
||||
// status. A failure recorded on a previous run must still be visible.
|
||||
await pumpCapture(
|
||||
tester,
|
||||
prefs: {
|
||||
'sync.lastTime': DateTime(2026, 7, 16, 12).toIso8601String(),
|
||||
'sync.lastOk': false,
|
||||
'sync.lastDetail': 'Exception: rate limited',
|
||||
},
|
||||
);
|
||||
await tester.pump();
|
||||
|
||||
expect(find.textContaining('Sync failed at 12:00:00'), findsOneWidget);
|
||||
expect(find.textContaining('rate limited'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('losing focus triggers a debounced auto-sync', (tester) async {
|
||||
final methods = <String>[];
|
||||
await pumpCapture(
|
||||
tester,
|
||||
prefs: configuredPrefs,
|
||||
httpClient: recordingMock(methods),
|
||||
);
|
||||
await tester.pump();
|
||||
await tester.pump();
|
||||
methods.clear();
|
||||
|
||||
tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.inactive);
|
||||
// A second signal (hidden also counts as focus loss) resets the debounce
|
||||
// instead of double-firing.
|
||||
tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.hidden);
|
||||
await tester.pump(
|
||||
CaptureScreen.autoSyncDebounce - const Duration(seconds: 1),
|
||||
);
|
||||
expect(methods, isEmpty); // still inside the debounce window
|
||||
|
||||
await tester.pump(const Duration(seconds: 1));
|
||||
await tester.pump();
|
||||
await tester.pump();
|
||||
|
||||
expect(methods, contains('PUT'));
|
||||
});
|
||||
|
||||
testWidgets('recovers notes from the local backup into an empty DB', (
|
||||
|
||||
@ -128,5 +128,28 @@ void main() {
|
||||
final result = await const SyncService().sync(local, _github(httpClient));
|
||||
expect(result.mergedDevices, 0); // both bad files skipped
|
||||
expect(putCalls, hasLength(1));
|
||||
// The skips are reported, not silent: that's how a peer device dropping
|
||||
// out of the merge becomes visible in the UI.
|
||||
expect(result.skippedFiles, ['bad1.json', 'bad2.json']);
|
||||
expect(result.toString(), contains('bad1.json'));
|
||||
});
|
||||
|
||||
test('a listed-but-unfetchable peer file is reported as skipped', () async {
|
||||
// The file appears in the directory listing but its GET 404s (deleted
|
||||
// between list and fetch, or contents unavailable).
|
||||
final (:httpClient, :putCalls) = _mockGitHub(
|
||||
contentResponses: {
|
||||
'todo-sync/notes': _response(200, [
|
||||
{'name': 'gone.json'},
|
||||
]),
|
||||
},
|
||||
);
|
||||
final local = await NoteRepository.openInMemory(nodeId: 'localNode');
|
||||
addTearDown(local.close);
|
||||
|
||||
final result = await const SyncService().sync(local, _github(httpClient));
|
||||
expect(result.mergedDevices, 0);
|
||||
expect(result.skippedFiles, ['gone.json']);
|
||||
expect(putCalls, hasLength(1)); // the push still happened
|
||||
});
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user