v0.13.0: the merge report says who, what, and where
The merge window kept reporting "Order kept from this machine (most recently
edited)" for playlists edited on the other machine. Three defects, confirmed
against the real snapshots in .resolved/:
* "this machine" was inferred from which copy held the plain filename. That is
Syncthing's call, not a statement about authorship — it sets the local copy
aside as readily as a remote one. In the 9:34 PM `* a fresh master` merge the
copy labelled "the other machine" was this machine's own 3:15 PM merge output,
so the label was exactly backwards. The 7-char device ID in the conflict
filename — the only real evidence — was matched by a bare \w+ and deleted with
the file. New sync_identity.py decodes it against Syncthing's config.xml and
works out which device is us from cert.pem.
* The decision leaned local. date_modified was only consulted when *both* copies
had one, and an iTunes playlist never reordered here has none — so the honest
comparison was skipped exactly when one machine had edited and the other
hadn't. A stamped copy now beats an unstamped one; mtime is the fallback only
when neither side has ever been edited. And every merge used to rewrite the
file it kept whether or not anything changed, freshening its mtime while the
conflict file kept its origin's: a ratchet. No-op merges write nothing, and a
merge whose result is a union neither copy had stamps date_modified, so the
other machine adopts it instead of trading the same 19 tracks back and forth.
* Nothing was actionable. Re-inserted tracks are now named with their position
("Pola — Abeille -> position 24, after ..."), six in the window and all of
them in what-changed.txt at the top of the backup snapshot, alongside the real
conflict filename and its device. Tracks only this copy has are reported too
rather than resurrected in silence.
Also fixed while in here: a rename or folder move made elsewhere was discarded
by every merge (only track_ids and settings were adopted); _reconcile_playlist
asserted the local edit was newer and never checked, so a reorder synced in from
the other machine was undone and flushed back to disk, and the branch reaching
it was gated on a dirty flag that a column drag sets; and _merge_metadata
decided the music folder from whichever copy an mtime coin flip had kept.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9ZEFi4qNJ39FMiBtiAxy2
This commit is contained in:
@@ -0,0 +1,382 @@
|
||||
"""Round 43: the merge report says who, what, and where.
|
||||
|
||||
The window kept reporting "Order kept from this machine (most recently edited)"
|
||||
for playlists that had been edited on the *other* machine, and offered a backup
|
||||
folder holding two directories with one hex-named JSON each.
|
||||
|
||||
Three defects behind that:
|
||||
|
||||
* "this machine" was inferred from which copy held the plain filename. Syncthing
|
||||
decides that, and it sets the local copy aside as readily as a remote one — so
|
||||
the label was a coin flip presented as a fact. The device ID in the conflict
|
||||
filename, the one piece of real evidence, was matched by a bare ``\\w+`` and
|
||||
deleted with the file.
|
||||
* The decision itself leaned local: ``date_modified`` was only consulted when
|
||||
*both* copies had one (an iTunes playlist never reordered here has none), and
|
||||
every merge rewrote the file it kept, so its mtime — the fallback — got
|
||||
fresher each time while the conflict file kept its origin's.
|
||||
* Nothing was actionable: ``merge_track_order`` knows where every re-inserted
|
||||
track landed and threw it away, no song was ever named, and a track the other
|
||||
copy had *deleted* came back silently.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
from lintunes import sync_identity
|
||||
from lintunes.models import Library, Playlist, PlaylistType
|
||||
from lintunes.storage import json_storage, conflict_resolver
|
||||
from lintunes.storage.conflict_resolver import CONFLICT_PATTERN, CHANGE, INFO
|
||||
|
||||
|
||||
def _conflict_name(original: str, device: str = "ABCDEFG") -> str:
|
||||
stem, _, ext = original.rpartition(".")
|
||||
return f"{stem}.sync-conflict-20240101-120000-{device}.{ext}"
|
||||
|
||||
|
||||
def _library(tmp_path, playlists=(), tracks=()):
|
||||
lib = Library()
|
||||
for p in playlists:
|
||||
lib.playlists[p.persistent_id] = p
|
||||
for track in tracks:
|
||||
lib.tracks[track.track_id] = track
|
||||
json_storage.save_library(lib, tmp_path)
|
||||
return lib
|
||||
|
||||
|
||||
def _playlist(ids, stamp=None, name="a nissa ideas"):
|
||||
pl = Playlist(persistent_id="AAAA1111", name=name,
|
||||
playlist_type=PlaylistType.REGULAR, track_ids=list(ids))
|
||||
pl.date_modified = stamp
|
||||
return pl
|
||||
|
||||
|
||||
def _pair(tmp_path, ours, theirs, our_stamp=None, their_stamp=None,
|
||||
device="ABCDEFG", their_name=None, tracks=()):
|
||||
"""Write our playlist plus a conflict file holding theirs."""
|
||||
_library(tmp_path, [_playlist(ours, our_stamp)], tracks)
|
||||
pfile = tmp_path / "playlists" / "AAAA1111.json"
|
||||
other = json.loads(pfile.read_text())
|
||||
other["track_ids"] = list(theirs)
|
||||
if their_stamp:
|
||||
other["date_modified"] = their_stamp
|
||||
elif "date_modified" in other:
|
||||
del other["date_modified"]
|
||||
if their_name:
|
||||
other["name"] = their_name
|
||||
cfile = tmp_path / "playlists" / _conflict_name("AAAA1111.json", device)
|
||||
cfile.write_text(json.dumps(other))
|
||||
return pfile, cfile
|
||||
|
||||
|
||||
class TestWhichCopyWon:
|
||||
"""The half-stamped case: one machine edited, the other never has."""
|
||||
|
||||
def test_the_stamped_copy_wins_over_an_unstamped_one(self, tmp_path):
|
||||
"""A stamp only exists once LinTunes recorded an edit, so "stamped vs
|
||||
never edited" is evidence — and it used to fall through to mtime, which
|
||||
is exactly the `sci vibes` case where their copy had no stamp at all."""
|
||||
pfile, cfile = _pair(tmp_path, ours=[1, 2, 3], theirs=[3, 2, 1],
|
||||
our_stamp=None, their_stamp="2026-08-20T18:00:00")
|
||||
os.utime(cfile, (time.time() - 10_000, time.time() - 10_000)) # older file
|
||||
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
assert json.loads(pfile.read_text())["track_ids"] == [3, 2, 1]
|
||||
assert "set aside" in summaries[0].lines[0]
|
||||
|
||||
def test_and_the_other_way_round(self, tmp_path):
|
||||
pfile, cfile = _pair(tmp_path, ours=[3, 2, 1], theirs=[1, 2, 3],
|
||||
our_stamp="2026-08-20T18:00:00", their_stamp=None)
|
||||
os.utime(cfile, (time.time() + 10_000, time.time() + 10_000)) # newer file
|
||||
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
assert json.loads(pfile.read_text())["track_ids"] == [3, 2, 1]
|
||||
assert "already here" in summaries[0].lines[0]
|
||||
|
||||
def test_neither_stamped_still_falls_back_to_mtime(self, tmp_path):
|
||||
pfile, cfile = _pair(tmp_path, ours=[1, 2, 3], theirs=[3, 2, 1])
|
||||
os.utime(cfile, (time.time() + 10_000, time.time() + 10_000))
|
||||
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
assert json.loads(pfile.read_text())["track_ids"] == [3, 2, 1]
|
||||
assert any("file timestamp" in line for line in summaries[0].lines)
|
||||
|
||||
|
||||
class TestTheMtimeRatchet:
|
||||
"""Every merge used to rewrite the file it kept, so the copy in place got a
|
||||
fresher mtime each round while the conflict file kept its origin's — the
|
||||
fallback got more biased with every merge."""
|
||||
|
||||
def test_a_merge_that_changes_nothing_does_not_rewrite_the_file(self, tmp_path):
|
||||
pfile, _ = _pair(tmp_path, ours=[1, 2, 3], theirs=[1, 2, 3])
|
||||
before = pfile.stat().st_mtime_ns
|
||||
time.sleep(0.01)
|
||||
|
||||
conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
assert pfile.stat().st_mtime_ns == before
|
||||
|
||||
def test_a_union_neither_copy_had_is_stamped_as_new(self, tmp_path):
|
||||
"""Content neither side had is newer than both. Saying so is what stops
|
||||
the two machines trading the same tracks back and forth."""
|
||||
pfile, _ = _pair(tmp_path, ours=[1, 2], theirs=[3],
|
||||
our_stamp="2026-08-20T18:00:00",
|
||||
their_stamp="2026-08-19T09:00:00")
|
||||
|
||||
conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
merged = json.loads(pfile.read_text())
|
||||
assert set(merged["track_ids"]) == {1, 2, 3}
|
||||
assert merged["date_modified"] > "2026-08-20T18:00:00"
|
||||
|
||||
def test_a_losing_copys_stamp_never_walks_ours_backwards(self, tmp_path):
|
||||
"""Their list can be a superset of ours while being the older edit. The
|
||||
merged file is still content we did not have — taking the loser's stamp
|
||||
would make this file claim to be older than it is and lose the next
|
||||
comparison for no reason."""
|
||||
pfile, _ = _pair(tmp_path, ours=[1], theirs=[1, 2],
|
||||
our_stamp="2026-08-20T18:00:00",
|
||||
their_stamp="2026-08-19T09:00:00")
|
||||
|
||||
conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
merged = json.loads(pfile.read_text())
|
||||
assert merged["track_ids"] == [1, 2]
|
||||
assert merged["date_modified"] > "2026-08-20T18:00:00"
|
||||
|
||||
def test_but_taking_their_list_wholesale_keeps_their_stamp(self, tmp_path):
|
||||
pfile, _ = _pair(tmp_path, ours=[1, 2], theirs=[1, 2, 3],
|
||||
our_stamp="2026-08-19T09:00:00",
|
||||
their_stamp="2026-08-20T18:00:00")
|
||||
|
||||
conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
assert json.loads(pfile.read_text())["date_modified"] == "2026-08-20T18:00:00"
|
||||
|
||||
|
||||
class TestWhoWroteIt:
|
||||
def test_the_pattern_captures_the_device_token(self):
|
||||
match = CONFLICT_PATTERN.match(
|
||||
"AAAA1111.sync-conflict-20240101-120000-CMFNCIX.json")
|
||||
assert match.group(1) == "AAAA1111"
|
||||
assert match.group(2) == "20240101-120000"
|
||||
assert match.group(3) == "CMFNCIX"
|
||||
assert match.group(4) == ".json"
|
||||
|
||||
def test_the_original_is_still_found_with_the_extension_in_group_four(
|
||||
self, tmp_path):
|
||||
_pair(tmp_path, ours=[1], theirs=[2])
|
||||
found = conflict_resolver._find_conflict_files(tmp_path)
|
||||
assert len(found) == 1
|
||||
assert found[0].original_path.name == "AAAA1111.json"
|
||||
assert found[0].device == "ABCDEFG"
|
||||
|
||||
def test_a_known_device_is_named_in_the_report(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(sync_identity, "_cache",
|
||||
{"names": {"CMFNCIX": "trave14"}, "self": "EA4TRYN"})
|
||||
_pair(tmp_path, ours=[1, 2], theirs=[2, 1], device="CMFNCIX",
|
||||
our_stamp="2026-08-20T18:00:00", their_stamp="2026-08-19T09:00:00")
|
||||
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
assert summaries[0].device_label == "trave14"
|
||||
assert any("trave14" in line for line in summaries[0].lines)
|
||||
|
||||
def test_our_own_device_is_named_as_this_machine(self, monkeypatch):
|
||||
monkeypatch.setattr(sync_identity, "_cache",
|
||||
{"names": {"EA4TRYN": "console"}, "self": "EA4TRYN"})
|
||||
assert sync_identity.label_for("EA4TRYN") == "console (this machine)"
|
||||
|
||||
def test_no_syncthing_config_means_no_claim_about_machines(
|
||||
self, tmp_path, monkeypatch):
|
||||
"""A machine without Syncthing installed, or with its config somewhere
|
||||
we don't look, must still merge — just without naming anyone."""
|
||||
monkeypatch.setenv("HOME", str(tmp_path / "elsewhere"))
|
||||
monkeypatch.setenv("XDG_STATE_HOME", str(tmp_path / "elsewhere" / "state"))
|
||||
sync_identity.reset_cache()
|
||||
try:
|
||||
assert sync_identity.device_names() == {}
|
||||
assert sync_identity.self_device_id() is None
|
||||
assert sync_identity.label_for("CMFNCIX") is None
|
||||
_pair(tmp_path, ours=[1, 2], theirs=[2, 1],
|
||||
our_stamp="2026-08-20T18:00:00", their_stamp="2026-08-19T09:00:00")
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
assert summaries[0].device_label == ""
|
||||
assert "written on" not in summaries[0].lines[0]
|
||||
finally:
|
||||
sync_identity.reset_cache()
|
||||
|
||||
|
||||
class TestNamingTheSongs:
|
||||
def _tracks(self):
|
||||
from lintunes.models import Track
|
||||
return [Track(track_id=1, name="Kid A", artist="Radiohead"),
|
||||
Track(track_id=2, name="Abeille", artist="Pola"),
|
||||
Track(track_id=3, name="morning.", artist="jinsang")]
|
||||
|
||||
def test_re_inserted_tracks_are_named_with_their_position(self, tmp_path):
|
||||
_pair(tmp_path, ours=[1, 3], theirs=[1, 2, 3], tracks=self._tracks(),
|
||||
our_stamp="2026-08-20T18:00:00", their_stamp="2026-08-19T09:00:00")
|
||||
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
body = "\n".join(summaries[0].lines)
|
||||
assert "Pola — Abeille" in body
|
||||
assert "position 2" in body
|
||||
assert "Radiohead — Kid A" in body # what it landed after
|
||||
|
||||
def test_tracks_the_other_copy_dropped_are_reported_not_hidden(self, tmp_path):
|
||||
"""A union resurrects a track deleted on the other machine. That used to
|
||||
happen in silence, so the deletion bounced back every sync with no clue
|
||||
why."""
|
||||
_pair(tmp_path, ours=[1, 2], theirs=[1], tracks=self._tracks(),
|
||||
our_stamp="2026-08-20T18:00:00", their_stamp="2026-08-19T09:00:00")
|
||||
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
body = "\n".join(summaries[0].lines)
|
||||
assert "the other copy did not have" in body
|
||||
assert "Pola — Abeille" in body
|
||||
assert "delete it here too" in body
|
||||
|
||||
def test_the_window_caps_the_list_and_the_backup_holds_all_of_it(self, tmp_path):
|
||||
from lintunes.models import Track
|
||||
tracks = [Track(track_id=i, name=f"Song {i}", artist="A") for i in range(1, 21)]
|
||||
_pair(tmp_path, ours=[1], theirs=list(range(1, 21)), tracks=tracks,
|
||||
our_stamp="2026-08-20T18:00:00", their_stamp="2026-08-19T09:00:00")
|
||||
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
named = [line for line in summaries[0].lines if line.startswith(" • ")]
|
||||
assert len(named) == conflict_resolver.TRACKS_TO_NAME
|
||||
assert any("…and 13 more" in line for line in summaries[0].lines)
|
||||
assert len([line for line in summaries[0].detail
|
||||
if line.startswith(" • ")]) == 19
|
||||
|
||||
def test_a_rename_on_the_winning_copy_is_adopted_and_reported(self, tmp_path):
|
||||
"""Only track_ids and settings were ever taken from the winner, so a
|
||||
rename or a folder move made elsewhere was discarded by every merge."""
|
||||
pfile, _ = _pair(tmp_path, ours=[1, 2], theirs=[2, 1],
|
||||
their_name="a nissa two",
|
||||
our_stamp="2026-08-19T09:00:00",
|
||||
their_stamp="2026-08-20T18:00:00")
|
||||
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
assert json.loads(pfile.read_text())["name"] == "a nissa two"
|
||||
assert summaries[0].file == "a nissa two"
|
||||
assert any("Renamed from" in line for line in summaries[0].lines)
|
||||
|
||||
|
||||
class TestTheBackupReport:
|
||||
def test_what_changed_is_written_beside_the_snapshots(self, tmp_path):
|
||||
from lintunes.models import Track
|
||||
_pair(tmp_path, ours=[1], theirs=[1, 2], device="CMFNCIX",
|
||||
tracks=[Track(track_id=1, name="Kid A", artist="Radiohead"),
|
||||
Track(track_id=2, name="Abeille", artist="Pola")],
|
||||
our_stamp="2026-08-20T18:00:00", their_stamp="2026-08-19T09:00:00")
|
||||
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
|
||||
report = (tmp_path / ".resolved" / summaries[0].backup_dir.rsplit("/", 1)[-1]
|
||||
/ "what-changed.txt").read_text()
|
||||
assert "a nissa ideas" in report
|
||||
assert "Pola — Abeille" in report
|
||||
# The conflict filename is the only record of the device, and the file
|
||||
# itself is deleted by the merge.
|
||||
assert "sync-conflict-20240101-120000-CMFNCIX.json" in report
|
||||
|
||||
def test_restore_still_only_touches_the_original_snapshot(self, tmp_path):
|
||||
pfile, _ = _pair(tmp_path, ours=[1, 2], theirs=[3],
|
||||
our_stamp="2026-08-20T18:00:00",
|
||||
their_stamp="2026-08-19T09:00:00")
|
||||
summaries = conflict_resolver.resolve_conflicts(tmp_path)
|
||||
assert set(json.loads(pfile.read_text())["track_ids"]) == {1, 2, 3}
|
||||
|
||||
from pathlib import Path
|
||||
conflict_resolver.restore_backup(Path(summaries[0].backup_dir), tmp_path)
|
||||
|
||||
assert json.loads(pfile.read_text())["track_ids"] == [1, 2]
|
||||
|
||||
|
||||
class TestTheLiveReloadPath:
|
||||
"""Same symptom with no conflict file: Syncthing delivers a clean update
|
||||
while we hold the playlist dirty."""
|
||||
|
||||
def _manager(self, tmp_path, qapp, ids, stamp):
|
||||
from lintunes.library_manager import LibraryManager
|
||||
lib = _library(tmp_path, [_playlist(ids, stamp)])
|
||||
return LibraryManager(lib, tmp_path)
|
||||
|
||||
def _write_disk_copy(self, tmp_path, ids, stamp, name=None):
|
||||
pfile = tmp_path / "playlists" / "AAAA1111.json"
|
||||
data = json.loads(pfile.read_text())
|
||||
data["track_ids"] = list(ids)
|
||||
data["date_modified"] = stamp
|
||||
if name:
|
||||
data["name"] = name
|
||||
pfile.write_text(json.dumps(data))
|
||||
|
||||
def test_a_newer_disk_reorder_is_not_reverted(self, tmp_path, qapp):
|
||||
manager = self._manager(tmp_path, qapp, [1, 2, 3], "2026-08-19T09:00:00")
|
||||
manager.library.playlists["AAAA1111"].track_ids = [1, 2, 3]
|
||||
manager._dirty_playlist_content.add("AAAA1111")
|
||||
self._write_disk_copy(tmp_path, [3, 2, 1], "2026-08-20T18:00:00")
|
||||
|
||||
manager.reload_from_disk()
|
||||
|
||||
assert manager.library.playlists["AAAA1111"].track_ids == [3, 2, 1]
|
||||
|
||||
def test_our_newer_edit_still_wins(self, tmp_path, qapp):
|
||||
manager = self._manager(tmp_path, qapp, [1, 2, 3], "2026-08-19T09:00:00")
|
||||
pl = manager.library.playlists["AAAA1111"]
|
||||
pl.track_ids = [3, 1, 2]
|
||||
pl.date_modified = "2026-08-21T10:00:00"
|
||||
manager._dirty_playlist_content.add("AAAA1111")
|
||||
self._write_disk_copy(tmp_path, [1, 2, 3], "2026-08-20T18:00:00")
|
||||
|
||||
manager.reload_from_disk()
|
||||
|
||||
assert manager.library.playlists["AAAA1111"].track_ids == [3, 1, 2]
|
||||
|
||||
def test_a_column_drag_does_not_make_us_the_edited_copy(self, tmp_path, qapp):
|
||||
"""`_dirty_playlists` is set by a sort click too, and that used to route
|
||||
the reload through the keep-local branch — reverting the other machine's
|
||||
rename and reorder, then flushing the revert back to disk."""
|
||||
manager = self._manager(tmp_path, qapp, [1, 2, 3], "2026-08-19T09:00:00")
|
||||
manager.mark_playlist_settings_dirty("AAAA1111")
|
||||
self._write_disk_copy(tmp_path, [3, 2, 1], "2026-08-20T18:00:00",
|
||||
name="renamed elsewhere")
|
||||
|
||||
manager.reload_from_disk()
|
||||
|
||||
playlist = manager.library.playlists["AAAA1111"]
|
||||
assert playlist.track_ids == [3, 2, 1]
|
||||
assert playlist.name == "renamed elsewhere"
|
||||
|
||||
def test_a_rename_now_records_an_edit(self, tmp_path, qapp):
|
||||
"""Without a stamp a rename is invisible to every merge."""
|
||||
manager = self._manager(tmp_path, qapp, [1], None)
|
||||
manager.rename_playlist("AAAA1111", "a nissa three")
|
||||
assert manager.library.playlists["AAAA1111"].date_modified
|
||||
assert "AAAA1111" in manager._dirty_playlist_content
|
||||
|
||||
def test_a_superset_on_disk_does_not_age_our_stamp(self, tmp_path, qapp):
|
||||
"""Same hazard as the conflict path: the disk copy can hold every track
|
||||
we have and still be the older edit."""
|
||||
manager = self._manager(tmp_path, qapp, [1], "2026-08-21T10:00:00")
|
||||
pl = manager.library.playlists["AAAA1111"]
|
||||
pl.track_ids = [1]
|
||||
manager._dirty_playlist_content.add("AAAA1111")
|
||||
self._write_disk_copy(tmp_path, [1, 2], "2026-08-20T18:00:00")
|
||||
|
||||
manager.reload_from_disk()
|
||||
|
||||
pl = manager.library.playlists["AAAA1111"]
|
||||
assert pl.track_ids == [1, 2]
|
||||
assert pl.date_modified > "2026-08-21T10:00:00"
|
||||
Reference in New Issue
Block a user