From fc65b8ab83348ef27031ae4d0421c04ec4a6ca3e Mon Sep 17 00:00:00 2001
From: npeter83 <npeter83+claude@gmail.com>
Date: Sat, 7 Nov 2026 21:48:16 +0100
Subject: [PATCH] fix(playlists): a refused push spends none of the push
 allowance

The cooldown was checked before the list was found pushable and before its sync claim, so Watch later
or a list another sync held used up the push allowance. It now counts only a push that runs. The
docstring that called a YouTube mirror unpushable is corrected: its local edits sync back.
---
 backend/app/routes/playlists.py    | 10 ++++---
 backend/tests/test_abuse_limits.py | 42 ++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/backend/app/routes/playlists.py b/backend/app/routes/playlists.py
index c2a1fe8b..62f825c8 100644
--- a/backend/app/routes/playlists.py
+++ b/backend/app/routes/playlists.py
@@ -261,8 +261,9 @@ def _check_push_cooldown(user: User) -> None:
 
 
 def _pushable(db: Session, user: User, playlist_id: int) -> "Playlist":
-    """A local playlist the user may push to YouTube (create or update). The built-in
-    Watch later and read-only YouTube mirrors are never pushable."""
+    """A playlist the user may push to YouTube: a local one (created there on its first push) or a
+    mirror of their own YouTube list (its local edits merged back). Only the built-in Watch later is
+    never pushable."""
     pl = _own_playlist(db, user, playlist_id)
     if pl.kind == "watch_later":
         raise _refused(400, "watch_later_not_pushable")
@@ -278,8 +279,8 @@ def push_plan(
     """Dry run: what a two-way 'Sync to YouTube' merge would do (what each side gains/loses) +
     the quota estimate, so the UI can show a confirm. For a linked playlist this reads the live
     YouTube state (cheap, 1 unit/page). Write scope is required — syncing is a YouTube write."""
-    _check_push_cooldown(user)
     pl = _pushable(db, user, playlist_id)
+    _check_push_cooldown(user)  # after the refusal: a list that cannot be pushed spends nothing
     try:
         with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PUSH):
             plan = plan_merge(db, user, pl)
@@ -299,13 +300,14 @@ def push(
     """Two-way sync a local playlist with YouTube (create it on first push, else 3-way merge so
     neither side loses an item). Refuses if the estimated quota exceeds what's left today, so a
     big sync can't strand half-applied. Write scope required (require_write_scope)."""
-    _check_push_cooldown(user)
     pl = _pushable(db, user, playlist_id)
     # One sync of a list at a time, across tabs, devices and the scheduler (`sync_claim`): the
     # client's own in-flight flag only covers the menus of one page.
     with sync_claim(db, pl.id) as free:
         if not free:
             raise _refused(409, "sync_busy")
+        # Spent only by a push that runs, as Revert's pull: a busy list or Watch later costs nothing.
+        _check_push_cooldown(user)
         try:
             with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PUSH):
                 # Read the live playlist ONCE (linked playlists only) and share it with both the
diff --git a/backend/tests/test_abuse_limits.py b/backend/tests/test_abuse_limits.py
index bcdc499b..ae30f3dd 100644
--- a/backend/tests/test_abuse_limits.py
+++ b/backend/tests/test_abuse_limits.py
@@ -177,6 +177,48 @@ def test_plan_and_push_draw_on_one_allowance(db, as_user, make_user, monkeypatch
     assert r.json()["detail"] == {"code": "playlist", "reason": "push_cooldown"}
 
 
+def test_a_refused_push_spends_no_allowance(db, as_user, make_user, monkeypatch):
+    # The allowance counts pushes that run: Watch later (never pushable) and a list another sync
+    # holds are refused before any YouTube call, so they must not leave the next real press at 429.
+    from contextlib import contextmanager
+
+    from app.reader.models import Playlist
+    from app.routes import playlists as plroutes
+
+    user, pl = _pushable_list(db, make_user, monkeypatch, plroutes)
+    wl = Playlist(user_id=user.id, name="Watch later", kind="watch_later")
+    db.add(wl)
+    db.commit()
+    c = as_user(user)
+    over = plroutes.PUSH_ALLOWANCE + 1
+    assert {c.get(f"/api/playlists/{wl.id}/push-plan").status_code for _ in range(over)} == {400}
+    assert {c.post(f"/api/playlists/{wl.id}/push").status_code for _ in range(over)} == {400}
+
+    @contextmanager
+    def busy(_db, _id):
+        yield False
+
+    with monkeypatch.context() as m:
+        m.setattr(plroutes, "sync_claim", busy)
+        assert {c.post(f"/api/playlists/{pl.id}/push").status_code for _ in range(over)} == {409}
+    assert c.post(f"/api/playlists/{pl.id}/push").status_code == 200
+
+
+def test_a_mirrored_youtube_list_is_pushable(db, as_user, make_user, monkeypatch):
+    # A mirror of the user's own YouTube list takes local edits (`dirty`) and syncs them back; only
+    # Watch later is refused.
+    from app.reader.models import Playlist
+    from app.routes import playlists as plroutes
+
+    user, _ = _pushable_list(db, make_user, monkeypatch, plroutes)
+    mirror = Playlist(
+        user_id=user.id, name="Mine", source="youtube", yt_playlist_id="PLx", dirty=True
+    )
+    db.add(mirror)
+    db.commit()
+    assert as_user(user).get(f"/api/playlists/{mirror.id}/push-plan").status_code == 200
+
+
 def test_the_push_cooldown_is_per_user(db, as_user, make_user, monkeypatch):
     from app.routes import playlists as plroutes