From 65c9948c78ca2796e151b3fa4112c0c49e0a895c Mon Sep 17 00:00:00 2001
From: BoulderBadgeDad <batmanesignal@gmail.com>
Date: Thu, 4 Jun 2026 15:57:40 -0700
Subject: [PATCH] Fix #792: reconcile sync mode was clamped back to 'replace'
 in the backend
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

start_playlist_sync validated the resolved mode with 'if sync_mode not in
(replace, append): sync_mode = replace' — a pre-existing clamp two lines below
the config read I added, which silently downgraded a configured 'reconcile' to
'replace'. So config=reconcile resolved correctly then got clobbered, and every
sync ran replace regardless of the setting (and incognito didn't help — it's
backend, not cache).

Replace the hand-rolled clamp with a pure normalize_sync_mode(requested,
configured) helper (VALID_SYNC_MODES includes reconcile) so the resolution is
testable and can't silently drop a mode again. Regression tests cover
reconcile-from-config, request-overrides-config, and unknown->replace.
---
 core/sync/playlist_edit.py  | 24 +++++++++++++++++++++++-
 tests/test_playlist_edit.py | 26 ++++++++++++++++++++++++++
 web_server.py               |  6 +++---
 3 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/core/sync/playlist_edit.py b/core/sync/playlist_edit.py
index 1ba650dc..80325238 100644
--- a/core/sync/playlist_edit.py
+++ b/core/sync/playlist_edit.py
@@ -109,4 +109,26 @@ def plan_playlist_reconcile(
     return {"add": add, "remove": remove}
 
 
-__all__ = ["plan_playlist_add", "remove_one_occurrence", "plan_playlist_reconcile"]
+VALID_SYNC_MODES = ("replace", "append", "reconcile")
+
+
+def normalize_sync_mode(requested, configured, default: str = "replace") -> str:
+    """Resolve the effective playlist sync mode.
+
+    An explicit per-request value wins; otherwise the configured default
+    (Settings > Playlist sync mode); anything unrecognized falls back to
+    ``default``. Keeping ``reconcile`` in ``VALID_SYNC_MODES`` is load-bearing —
+    a validation list that omits it silently downgrades reconcile to replace,
+    which is exactly the #792 regression this helper exists to prevent.
+    """
+    mode = (requested or "") or (configured or "") or default
+    return mode if mode in VALID_SYNC_MODES else default
+
+
+__all__ = [
+    "plan_playlist_add",
+    "remove_one_occurrence",
+    "plan_playlist_reconcile",
+    "normalize_sync_mode",
+    "VALID_SYNC_MODES",
+]
diff --git a/tests/test_playlist_edit.py b/tests/test_playlist_edit.py
index a75ff8d6..8cc0e10d 100644
--- a/tests/test_playlist_edit.py
+++ b/tests/test_playlist_edit.py
@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 from core.sync.playlist_edit import (
+    normalize_sync_mode,
     plan_playlist_add,
     plan_playlist_reconcile,
     remove_one_occurrence,
@@ -126,3 +127,28 @@ def test_reconcile_duplicate_still_desired_not_removed():
     # a gone id is listed once even if it had duplicates.
     plan = plan_playlist_reconcile(['a', 'a', 'b', 'b'], ['a'])
     assert plan == {'add': [], 'remove': ['b']}
+
+
+# ── normalize_sync_mode: reconcile must survive resolution (#792 regression) ──
+
+def test_normalize_keeps_reconcile_from_config():
+    # The bug: a validation list omitting 'reconcile' downgraded the configured
+    # setting to 'replace'. No per-request override → configured value wins.
+    assert normalize_sync_mode(None, 'reconcile') == 'reconcile'
+    assert normalize_sync_mode('', 'reconcile') == 'reconcile'
+
+
+def test_normalize_request_overrides_config():
+    assert normalize_sync_mode('append', 'reconcile') == 'append'
+    assert normalize_sync_mode('reconcile', 'replace') == 'reconcile'
+
+
+def test_normalize_falls_back_for_unknown():
+    assert normalize_sync_mode('bogus', 'replace') == 'replace'
+    assert normalize_sync_mode(None, None) == 'replace'
+    assert normalize_sync_mode(None, 'also-bogus') == 'replace'
+
+
+def test_normalize_all_real_modes_pass_through():
+    for m in ('replace', 'append', 'reconcile'):
+        assert normalize_sync_mode(m, 'replace') == m
diff --git a/web_server.py b/web_server.py
index 72ac4a6b..f4a897ce 100644
--- a/web_server.py
+++ b/web_server.py
@@ -23548,9 +23548,9 @@ def start_playlist_sync():
     # /Playlists/<id>/Items, Navidrome updatePlaylist?songIdToAdd=...).
     # Per-request sync_mode wins; otherwise use the configured default
     # (Settings > Playlist sync mode). Default 'replace' keeps today's behavior.
-    sync_mode = data.get('sync_mode') or config_manager.get('playlist_sync.mode', 'replace')
-    if sync_mode not in ('replace', 'append'):
-        sync_mode = 'replace'
+    from core.sync.playlist_edit import normalize_sync_mode
+    sync_mode = normalize_sync_mode(data.get('sync_mode'),
+                                    config_manager.get('playlist_sync.mode', 'replace'))
 
     if not all([playlist_id, playlist_name, tracks_json]):
         return jsonify({"success": False, "error": "Missing playlist_id, name, or tracks."}), 400