mirror of
https://github.com/kuhyx/todo-app.git
synced 2026-11-10 20:27:01 +01:00
Restore notes at their own edit time, not "now"
Backup recovery and file import both wrote restored notes through `upsert`, which stamps a fresh clock. Under crdt_sync's per-field last-writer-wins that makes a *restore* outrank every other device: the restoring device claims it edited every field just now, so it silently overwrites newer edits made elsewhere on the next sync. This is not hypothetical. The new desktop app seeded itself from BACKLOG.md and rewrote 245 of 330 field clocks to today under a fresh node id, then synced -- taking authority over the phone for ~74% of all fields. Content happened to survive (the copy came from the same data; only a trailing newline on 5 notes differed via the Markdown round-trip), but nothing about the mechanism guaranteed that. The legacy sqlite migration already did the right thing for exactly this reason, so the fix is to use that same helper everywhere a note is *restored* rather than newly written. Renamed legacyRecordFor -> recordAtNoteTime, since it is no longer migration-specific. The freshness check in importNotes is unchanged: it decides whether the import wins *locally*. The clock decides whether it wins on *other* devices, and an import must not win there merely by being recent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
78452bf995
commit
313368530a
@ -42,7 +42,7 @@ class LegacySqliteMigration {
|
||||
final log = <String, Record>{};
|
||||
for (final row in rows) {
|
||||
final note = Note.fromRow(row);
|
||||
log[note.id] = NoteRepository.legacyRecordFor(
|
||||
log[note.id] = NoteRepository.recordAtNoteTime(
|
||||
note,
|
||||
nodeId,
|
||||
deleted: (row['is_deleted'] as int? ?? 0) != 0,
|
||||
|
||||
@ -115,14 +115,20 @@ class NoteRepository {
|
||||
return NoteRepository._(store, nodeId);
|
||||
}
|
||||
|
||||
/// Builds the record for one migrated legacy note, tombstones included.
|
||||
/// Builds a record whose field clocks come from the note's own `updatedAt`
|
||||
/// rather than from "now".
|
||||
///
|
||||
/// Each note's field clocks are seeded from its own `updated_at`, not "now",
|
||||
/// so that when two devices migrate independently, a pre-cutover edit still
|
||||
/// wins by real edit time under per-field last-writer-wins instead of being
|
||||
/// resolved arbitrarily. Public because the migration that calls it lives in
|
||||
/// the io-only library, which cannot reach private members here.
|
||||
static Record legacyRecordFor(
|
||||
/// Used by every path that *restores* notes the user already had — the
|
||||
/// legacy sqlite migration, backup recovery, and file import. Stamping "now"
|
||||
/// there is actively harmful: under per-field last-writer-wins, a restored
|
||||
/// copy would outrank the same note on every other device, so a device that
|
||||
/// recovered from a backup would silently overwrite newer edits made
|
||||
/// elsewhere. Seeding from real edit time makes a restore lose to genuinely
|
||||
/// newer data, which is what a restore should do.
|
||||
///
|
||||
/// Public because the legacy migration lives in an io-only library that
|
||||
/// cannot reach private members here.
|
||||
static Record recordAtNoteTime(
|
||||
Note note,
|
||||
String nodeId, {
|
||||
bool deleted = false,
|
||||
@ -169,11 +175,15 @@ class NoteRepository {
|
||||
var skipped = 0;
|
||||
for (final note in incoming) {
|
||||
final local = existing[note.id];
|
||||
// Restore at the note's own edit time, never "now" — see
|
||||
// [recordAtNoteTime]. The freshness check above is what decides whether
|
||||
// the import wins locally; the clock decides whether it wins on *other*
|
||||
// devices, and an import must not win there simply by being recent.
|
||||
if (local == null) {
|
||||
await upsert(note);
|
||||
await _store.upsert(recordAtNoteTime(note, _nodeId));
|
||||
added++;
|
||||
} else if (note.updatedAt.isAfter(local.updatedAt)) {
|
||||
await upsert(note);
|
||||
await _store.upsert(recordAtNoteTime(note, _nodeId));
|
||||
updated++;
|
||||
} else {
|
||||
skipped++;
|
||||
|
||||
@ -243,6 +243,29 @@ void main() {
|
||||
});
|
||||
|
||||
group('importNotes (safe merge)', () {
|
||||
test('restores at the note\'s own edit time, not "now"', () async {
|
||||
// Regression guard. Recovery used to stamp fresh clocks on every
|
||||
// restored note, so a device that recovered from a backup outranked
|
||||
// every other device under per-field last-writer-wins and silently
|
||||
// overwrote newer edits made elsewhere. Observed for real: a restored
|
||||
// desktop rewrote 245 of 330 field clocks to "now".
|
||||
final repo = await NoteRepository.openInMemory();
|
||||
addTearDown(repo.close);
|
||||
final edited = DateTime(2026, 3, 4, 5, 6, 7);
|
||||
|
||||
await repo.importNotes([
|
||||
note('restored', 'from a backup', createdAt: edited, updatedAt: edited),
|
||||
]);
|
||||
|
||||
final field = repo.exportLog()['restored']!.fields['text']!;
|
||||
expect(
|
||||
field.$2.wallTimeMs,
|
||||
edited.millisecondsSinceEpoch,
|
||||
reason: 'clock must come from updatedAt so a restore cannot outrank '
|
||||
'genuinely newer data on another device',
|
||||
);
|
||||
});
|
||||
|
||||
test('adds notes whose id is not present locally', () async {
|
||||
final repo = await NoteRepository.openInMemory();
|
||||
addTearDown(repo.close);
|
||||
|
||||
Loading…
Reference in New Issue
Block a user