diff --git a/CLAUDE.md b/CLAUDE.md index 70bc60c6..f8f7bf40 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -82,8 +82,8 @@ CQRS, a repository/DI framework, a router library, a generic CRUD-router factory | Admin gate import | `app.auth` | the `app.routes.admin` re-export | | Upstream failure status | `422` with JSON detail | `502` (reserve 502 for raw byte-proxies) | | User-facing error | structured `detail` (`{code, reason}`) | a bare English string under a bilingual UI | -| Mutation response | `{"ok": true, …}` (`{"deleted": id}`) | ad-hoc `{"unsubscribed"}`/`{"cleared": n}`/echo | -| Transaction commit | the handler/job-wrapper commits; helpers `flush` | helpers that commit (only `quota` owns its session) | +| Mutation response | `{"ok": true, …}` (`{"deleted": id}`), test-held | a bare `{"unsubscribed"}`/`{"cleared": n}`, no `ok` | +| Transaction commit | the handler commits; helpers `flush` (test-held list) | helpers that commit (only `quota` owns its session) | | Admin knob storage | `sysconfig` (values) | env / `app_state` / `scheduler_settings` scatter | | Job status word | the `downloads/status.py` StrEnum | loose `"canceled"`/`"cancelled"` literals | | Current time | inline `datetime.now(timezone.utc)` | new `now_utc`/`_now` helpers | diff --git a/backend/app/routes/admin.py b/backend/app/routes/admin.py index c77f40b3..43fad55d 100644 --- a/backend/app/routes/admin.py +++ b/backend/app/routes/admin.py @@ -170,7 +170,7 @@ def _activate_user_by_email(db: Session, email: str) -> None: user = next((u for u in candidates if u.email == email), None) or next(iter(candidates), None) if user is not None and not user.is_active: user.is_active = True - db.commit() + db.flush() # the handler commits (CLAUDE.md: helpers flush) def _decide(db: Session, invite_id: int, admin: User, approved: bool) -> Invite: @@ -192,7 +192,7 @@ def _decide(db: Session, invite_id: int, admin: User, approved: bool) -> Invite: target_id=inv.id, summary=inv.email, ) - db.commit() + db.flush() # the handler commits, with the activation that goes with it return inv @@ -211,6 +211,9 @@ def approve_invite( link, has_account, hours = _welcome_mail_parts(db, inv.email) lang = mailtext.lang_for_email(db, inv.email) background.add_task(email_mod.send_access_approved, inv.email, lang, link, has_account, hours) + # Before the response, so the mail (a background task, after it) never describes a decision + # the database does not hold yet. + db.commit() return _serialize(inv, has_account=bool(_accounts_for(db, [inv.email]))) @@ -227,6 +230,7 @@ def deny_invite( background.add_task( email_mod.send_access_denied, inv.email, mailtext.lang_for_email(db, inv.email) ) + db.commit() return _serialize(inv, has_account=bool(_accounts_for(db, [inv.email]))) @@ -265,6 +269,7 @@ def add_invite( ) db.commit() _activate_user_by_email(db, email) + db.commit() return _serialize(inv, has_account=bool(_accounts_for(db, [email]))) @@ -831,7 +836,7 @@ def reset_demo(admin: User = Depends(admin_user), db: Session = Depends(get_db)) db, admin.id, AuditAction.DEMO_RESET, summary=f"{seeded} sample playlists re-seeded" ) db.commit() - return {"reset": True, "playlists_seeded": seeded} + return {"ok": True, "reset": True, "playlists_seeded": seeded} @router.post("/purge-discovery") diff --git a/backend/app/routes/audit.py b/backend/app/routes/audit.py index 6562e3cf..d9a907b9 100644 --- a/backend/app/routes/audit.py +++ b/backend/app/routes/audit.py @@ -68,4 +68,4 @@ def clear_audit(admin: User = Depends(admin_user), db: Session = Depends(get_db) n = db.execute(delete(AuditLog)).rowcount or 0 audit.record(db, admin.id, AuditAction.AUDIT_CLEAR, summary=f"Cleared {n} audit entries") db.commit() - return {"cleared": n} + return {"ok": True, "cleared": n} diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index 0b614648..c3bbd32f 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -673,7 +673,7 @@ def demo_login(body: EmailIn, request: Request, db: Session = Depends(get_db)) - exactly like a non-match (authenticated=false) so the field can't be hammered as an enumeration oracle. On a match it sets the session and the client reloads into the app.""" if not _demo_limiter.allow(client_key(request)): - return {"authenticated": False} + return {"ok": True, "authenticated": False} email = (body.email or "").strip().lower() if valid_email(email) and is_demo_allowed(db, email): demo = get_or_create_demo_user(db) @@ -684,8 +684,8 @@ def demo_login(body: EmailIn, request: Request, db: Session = Depends(get_db)) - _remember_epoch(request, demo) # The demo key IS an address, so it is not in the line — see the denied branch above. log.info("signed in", extra=logs.event("auth.login.ok", method="demo", uid=demo.id)) - return {"authenticated": True} - return {"authenticated": False} + return {"ok": True, "authenticated": True} + return {"ok": True, "authenticated": False} def _establish_session(request: Request, user: User) -> None: @@ -822,14 +822,14 @@ def register( if not sysconfig.get_bool(db, "allow_registration"): raise refused(403, "registration_closed") if not _register_limiter.allow(client_key(request)): - return {"status": "ok"} # silently throttle; uniform response + return {"ok": True, "status": "ok"} # silently throttle; uniform response # Do the existence check + account creation OFF the response path (a background task with its own # session). The create path hashes the password + writes several rows + schedules emails; doing it # inline would make a NEW email respond measurably slower than an already-registered one (which # skips all that) — a timing oracle for enumeration. Off-path, any valid email responds the same. background.add_task(_register_account, email, password, body.lang) - return {"status": "ok"} + return {"ok": True, "status": "ok"} def _register_account(email: str, password: str, lang: str | None = None) -> None: @@ -995,11 +995,11 @@ def verify_email_resend( mail had no way back. Uniform response + timing regardless of whether the email has a pending account (the lookup runs off the response path), so it can't probe for registered emails.""" if not _verify_resend_limiter.allow(client_key(request)): - return {"status": "ok"} # silently throttle; uniform response + return {"ok": True, "status": "ok"} # silently throttle; uniform response email = (body.email or "").strip().lower() if valid_email(email): background.add_task(_resend_verify_if_eligible, email) - return {"status": "ok"} + return {"ok": True, "status": "ok"} def _resend_verify_if_eligible(email: str) -> None: @@ -1089,14 +1089,14 @@ def password_reset_request( """Request a password-reset link. Uniform response + timing regardless of whether the email has a password account (the lookup runs off the response path), so it can't probe for registered emails.""" if not _reset_limiter.allow(client_key(request)): - return {"status": "ok"} + return {"ok": True, "status": "ok"} email = (body.email or "").strip().lower() # Do the account lookup + token issue + email OFF the response path (a background task with its # own session), so the endpoint takes the same time for any valid email whether or not it has a # password account — no timing-based enumeration. Any valid email schedules the same task. if valid_email(email): background.add_task(_send_reset_if_eligible, email) - return {"status": "ok"} + return {"ok": True, "status": "ok"} def _send_reset_if_eligible(email: str) -> None: diff --git a/backend/app/routes/channels.py b/backend/app/routes/channels.py index 87e1623d..5fdd68e8 100644 --- a/backend/app/routes/channels.py +++ b/backend/app/routes/channels.py @@ -424,6 +424,7 @@ def explore_channel( except YouTubeError as exc: raise youtube_refusal(exc) return { + "ok": True, "channel_id": channel_id, "ingested": result["ingested"], "next_page_token": result["next_page_token"], @@ -505,6 +506,7 @@ def update_channel( background.add_task(backfill_channel_now, channel_id, user.id) return { + "ok": True, "id": channel_id, "priority": sub.priority, "hidden": sub.hidden, @@ -539,7 +541,7 @@ def reset_backfill( db.commit() with quota.attribute(user.id, quota.QuotaAction.VIDEOS_BACKFILL_RECENT): run_recent_backfill(db, [channel], max_channels=1) - return {"id": channel_id, "reset": True} + return {"ok": True, "id": channel_id, "reset": True} @router.post("/{channel_id}/subscribe") @@ -618,7 +620,7 @@ def subscribe( .values(via_explore=False) ) db.commit() - return {"subscribed": channel_id} + return {"ok": True, "subscribed": channel_id} @router.delete("/{channel_id}/subscription") @@ -645,7 +647,7 @@ def unsubscribe( raise youtube_refusal(exc) db.delete(sub) db.commit() - return {"unsubscribed": channel_id} + return {"ok": True, "unsubscribed": channel_id} @router.get("/blocked/list") @@ -686,7 +688,7 @@ def block_channel( ) ) db.commit() - return {"blocked": channel_id} + return {"ok": True, "blocked": channel_id} @router.delete("/{channel_id}/block") @@ -701,7 +703,7 @@ def unblock_channel( ) ) db.commit() - return {"unblocked": channel_id} + return {"ok": True, "unblocked": channel_id} class AttachTagIn(BaseModel): @@ -736,7 +738,7 @@ def attach_tag( ) ) db.commit() - return {"channel_id": channel_id, "tag_id": body.tag_id} + return {"ok": True, "channel_id": channel_id, "tag_id": body.tag_id} @router.delete("/{channel_id}/tags/{tag_id}") @@ -756,4 +758,4 @@ def detach_tag( if row is not None: db.delete(row) db.commit() - return {"channel_id": channel_id, "tag_id": tag_id} + return {"ok": True, "channel_id": channel_id, "tag_id": tag_id} diff --git a/backend/app/routes/config.py b/backend/app/routes/config.py index b5f68bf7..fbbc0e24 100644 --- a/backend/app/routes/config.py +++ b/backend/app/routes/config.py @@ -130,4 +130,4 @@ def send_test_email( sent = email_mod.send_test(user.email, mailtext.lang_for_user(user)) if not sent: raise refused(422, "smtp_failed") - return {"sent": True, "to": user.email} + return {"ok": True, "sent": True, "to": user.email} diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py index 20dc24ff..90e36197 100644 --- a/backend/app/routes/downloads.py +++ b/backend/app/routes/downloads.py @@ -847,7 +847,7 @@ def share_download( commit=False, ) db.commit() - return {"shared_with": recipient.email} + return {"ok": True, "shared_with": recipient.email} @router.delete("/{job_id}/share/{email}") @@ -891,7 +891,7 @@ def remove_shared_with_me( if row is not None: db.delete(row) db.commit() - return {"removed": job_id} + return {"ok": True, "removed": job_id} # --- recipients (for the internal "share with a user" picker) ------------------------------ diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index 940199c8..fb5edcb3 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -261,7 +261,7 @@ def set_video_state( # or moved this row. The state the caller asked for is the state that holds either way. except StaleDataError: db.rollback() - return {"video_id": video_id, "status": "new"} + return {"ok": True, "video_id": video_id, "status": "new"} # watched / hidden: atomic upsert. The player marks "watched" at end-of-playback at # the same instant a progress checkpoint may DELETE the in-progress row; a plain @@ -282,7 +282,7 @@ def set_video_state( ) db.execute(stmt) db.commit() - return {"video_id": video_id, "status": status} + return {"ok": True, "video_id": video_id, "status": status} @router.delete("/videos/{video_id}/state") @@ -304,7 +304,7 @@ def clear_video_state( # silent-ok: the row went under the delete, which is the outcome the delete wanted. except StaleDataError: db.rollback() - return {"video_id": video_id, "status": "new", "position_seconds": 0} + return {"ok": True, "video_id": video_id, "status": "new", "position_seconds": 0} class VideoProgressIn(BaseModel): @@ -362,7 +362,7 @@ def set_video_progress( # silent-ok: exactly the race the comment above describes; the response stays true. except StaleDataError: db.rollback() - return {"video_id": video_id, "position_seconds": 0} + return {"ok": True, "video_id": video_id, "position_seconds": 0} if row is None: row = VideoState(user_id=user.id, video_id=video_id) @@ -370,7 +370,7 @@ def set_video_progress( row.position_seconds = position row.progress_updated_at = datetime.now(timezone.utc) db.commit() - return {"video_id": video_id, "position_seconds": position} + return {"ok": True, "video_id": video_id, "position_seconds": position} @router.get("/videos/{video_id}") diff --git a/backend/app/routes/me.py b/backend/app/routes/me.py index ce6fab67..6f1784cf 100644 --- a/backend/app/routes/me.py +++ b/backend/app/routes/me.py @@ -214,7 +214,7 @@ def update_preferences( user.preferences = merged db.add(user) db.commit() - return {"preferences": merged} + return {"ok": True, "preferences": merged} class DiagnosticsIn(BaseModel): diff --git a/backend/app/routes/messages.py b/backend/app/routes/messages.py index 64156870..1cbe96fd 100644 --- a/backend/app/routes/messages.py +++ b/backend/app/routes/messages.py @@ -166,7 +166,7 @@ def setup_keys( ) ) db.commit() - return {"configured": True} + return {"ok": True, "configured": True} @router.post("/keys/reset") @@ -204,7 +204,7 @@ def reset_keys( if key is not None: db.delete(key) db.commit() - return {"configured": False} + return {"ok": True, "configured": False} @router.get("/keys/{user_id}") diff --git a/backend/app/routes/playlists.py b/backend/app/routes/playlists.py index 1256a6e1..95a0b2b1 100644 --- a/backend/app/routes/playlists.py +++ b/backend/app/routes/playlists.py @@ -106,7 +106,7 @@ def _add_item(db: Session, pl: Playlist, video_id: str, *, mark_dirty: bool) -> if mark_dirty: recompute_dirty(db, pl) _touch(pl) - db.commit() + db.flush() # the handler commits (CLAUDE.md: helpers flush) return True @@ -124,7 +124,7 @@ def _remove_item(db: Session, pl: Playlist, video_id: str, *, mark_dirty: bool) if mark_dirty: recompute_dirty(db, pl) _touch(pl) - db.commit() + db.flush() # the handler commits (CLAUDE.md: helpers flush) return True @@ -278,7 +278,7 @@ def _watch_later(db: Session, user: User) -> Playlist: position=_next_playlist_position(db, user.id), ) db.add(pl) - db.commit() + db.flush() # the id, for the item the caller adds next; the handler commits return pl @@ -293,7 +293,8 @@ def add_watch_later( get_or_404(db, Video, video_id, what="video") pl = _watch_later(db, user) _add_item(db, pl, video_id, mark_dirty=False) - return {"saved": True, "playlist_id": pl.id} + db.commit() + return {"ok": True, "saved": True, "playlist_id": pl.id} @router.delete("/watch-later/{video_id}") @@ -307,7 +308,8 @@ def remove_watch_later( ) if pl is not None: _remove_item(db, pl, video_id, mark_dirty=False) - return {"saved": False} + db.commit() + return {"ok": True, "saved": False} @router.post("/sync-youtube") @@ -516,7 +518,8 @@ def add_item( video_id = body.video_id get_or_404(db, Video, video_id, what="video") _add_item(db, pl, video_id, mark_dirty=True) - return {"playlist_id": pl.id, "video_id": video_id} + db.commit() + return {"ok": True, "playlist_id": pl.id, "video_id": video_id} @router.delete("/{playlist_id}/items/{video_id}") @@ -528,7 +531,8 @@ def remove_item( ) -> dict: pl = owned_or_404(db, Playlist, playlist_id, user, what="playlist") _remove_item(db, pl, video_id, mark_dirty=True) - return {"playlist_id": pl.id, "video_id": video_id} + db.commit() + return {"ok": True, "playlist_id": pl.id, "video_id": video_id} class MoveItemIn(BaseModel): @@ -641,4 +645,4 @@ def reorder_items( recompute_dirty(db, pl) _touch(pl) db.commit() - return {"playlist_id": pl.id, "count": pos} + return {"ok": True, "playlist_id": pl.id, "count": pos} diff --git a/backend/app/routes/plex/admin.py b/backend/app/routes/plex/admin.py index e9a2af74..f724bc7f 100644 --- a/backend/app/routes/plex/admin.py +++ b/backend/app/routes/plex/admin.py @@ -149,7 +149,7 @@ def watch_link_set( result: dict = {} if enabled and not link.initial_import_done: result = plex_watch.import_owner_watch_state(db, link) - return {**_link_dict(link), "import": result or None} + return {"ok": True, **_link_dict(link), "import": result or None} @router.post("/watch/import") @@ -160,4 +160,4 @@ def watch_import_now(user: User = Depends(admin_user), db: Session = Depends(get if link is None or not link.sync_enabled: raise refused(409, "watch_sync_off") result = plex_watch.import_owner_watch_state(db, link) - return {**_link_dict(link), "import": result} + return {"ok": True, **_link_dict(link), "import": result} diff --git a/backend/app/routes/plex/playback.py b/backend/app/routes/plex/playback.py index 4114b6e5..635c2b47 100644 --- a/backend/app/routes/plex/playback.py +++ b/backend/app/routes/plex/playback.py @@ -385,7 +385,7 @@ def item_progress( # item — or a second one racing this one — would otherwise bump Plex's viewCount again. if flipped: _push_watch(background, db, user.id, it.id, rating_key, "watched") - return {"status": "watched", "position_seconds": 0} + return {"ok": True, "status": "watched", "position_seconds": 0} st = ( db.execute( @@ -402,7 +402,7 @@ def item_progress( st.position_seconds = 0 st.progress_updated_at = now db.commit() - return {"position_seconds": 0} + return {"ok": True, "position_seconds": 0} if st is None: st = PlexState(user_id=user.id, item_id=it.id) @@ -427,7 +427,7 @@ def item_progress( duration * 1000, intent=now, ) - return {"position_seconds": position} + return {"ok": True, "position_seconds": position} class ItemStateIn(BaseModel): @@ -473,7 +473,7 @@ def item_state( db.commit() if mirrored_on_plex: _push_watch(background, db, user.id, it.id, rating_key, "unwatched") - return {"status": "new"} + return {"ok": True, "status": "new"} if st is None: st = PlexState(user_id=user.id, item_id=it.id) db.add(st) @@ -485,7 +485,7 @@ def item_state( db.commit() if status == "watched" and prev_status != "watched": _push_watch(background, db, user.id, it.id, rating_key, "watched") - return {"status": status} + return {"ok": True, "status": status} def _bulk_state( @@ -527,7 +527,9 @@ def _bulk_state( db.delete(st) if prev != "new": to_push.append((e.id, e.rating_key, "unwatched")) - db.commit() + # Flush, not commit: the handler commits, and it does so before the response — which is when + # the background push below runs, so the push always reads the committed marks. + db.flush() if pushable and to_push: # One coalesced background task for the whole batch (reuses a single session + Plex client), # not one task per episode — a 200-ep show mark is 1 task, not 200. @@ -558,7 +560,8 @@ def show_state( .all() ) changed = _bulk_state(background, db, user.id, eps, watched) - return {"changed": changed, "watched": watched} + db.commit() + return {"ok": True, "changed": changed, "watched": watched} @router.post("/season/{rating_key}/state") @@ -580,7 +583,8 @@ def season_state( .all() ) changed = _bulk_state(background, db, user.id, eps, watched) - return {"changed": changed, "watched": watched} + db.commit() + return {"ok": True, "changed": changed, "watched": watched} @router.post("/stream/{rating_key}/session") @@ -605,6 +609,7 @@ def stream_session( # the client can switch audio without a restart — even for otherwise direct-playable files. if it.playable == "direct" and audio is None and not aoff and not multi: return { + "ok": True, "mode": "direct", "url": f"/api/plex/stream/{it.rating_key}/file", "start_s": 0.0, @@ -620,6 +625,7 @@ def stream_session( # surface the active mode + encoder in the stats overlay (`tonemap` = the HW HDR→SDR path); both # segment streams are otherwise identical. return { + "ok": True, "mode": "hls", "start_s": s.media_start_s, # The session id is part of the path (not a query param) so the playlist's RELATIVE segment diff --git a/backend/app/routes/plex/playlists.py b/backend/app/routes/plex/playlists.py index 9cd61fd5..8786584d 100644 --- a/backend/app/routes/plex/playlists.py +++ b/backend/app/routes/plex/playlists.py @@ -325,7 +325,7 @@ def playlist_add_bulk( ) or 0 ) - return {"added": added, "item_count": total} + return {"ok": True, "added": added, "item_count": total} @router.post("/playlists/{pid}/items/remove-bulk") @@ -356,7 +356,7 @@ def playlist_remove_bulk( ) or 0 ) - return {"removed": removed, "item_count": total} + return {"ok": True, "removed": removed, "item_count": total} @router.put("/playlists/{pid}/order") diff --git a/backend/app/routes/public.py b/backend/app/routes/public.py index 7dafd1fb..fa2a2107 100644 --- a/backend/app/routes/public.py +++ b/backend/app/routes/public.py @@ -107,6 +107,7 @@ def watch_unlock( # Not a password link — nothing to unlock; just hand back the plain meta. job, asset = _ready_target(db, link) return { + "ok": True, "needs_password": False, **linksmod.public_meta(link, job, asset, _file_url(token), _poster_url(token, asset)), } @@ -118,6 +119,7 @@ def watch_unlock( db.commit() grant = linksmod.make_grant(token, link.password_version) return { + "ok": True, "needs_password": False, **linksmod.public_meta( link, job, asset, _file_url(token, grant), _poster_url(token, asset, grant) diff --git a/backend/app/routes/scheduler.py b/backend/app/routes/scheduler.py index 70e786db..2a84dddc 100644 --- a/backend/app/routes/scheduler.py +++ b/backend/app/routes/scheduler.py @@ -60,7 +60,7 @@ def set_job_interval( after={"interval_minutes": applied}, ) db.commit() - return {"id": job_id, "interval_minutes": applied} + return {"ok": True, "id": job_id, "interval_minutes": applied} @router.post("/jobs/{job_id}/run") @@ -77,13 +77,13 @@ def run_job_now( raise not_found("job") if result == "already_running": raise refused(409, "job_running") - return {"id": job_id, "started": True} + return {"ok": True, "id": job_id, "started": True} @router.post("/run-all") def run_all_now(user: User = Depends(admin_user)) -> dict: """Trigger every job that isn't already running. Returns the ids actually started.""" - return {"started": trigger_all(actor_id=user.id)} + return {"ok": True, "started": trigger_all(actor_id=user.id)} class MaintenanceSettingsIn(BaseModel): @@ -112,7 +112,7 @@ def set_maintenance_settings( after={"revalidate_batch": new}, ) db.commit() - return {"revalidate_batch": new} + return {"ok": True, "revalidate_batch": new} @router.get("") diff --git a/backend/app/routes/search.py b/backend/app/routes/search.py index 3936dbaf..7fa9af3b 100644 --- a/backend/app/routes/search.py +++ b/backend/app/routes/search.py @@ -411,7 +411,7 @@ def clear_search( find. Kept videos and organically-owned ones are left untouched.""" ids = body.video_ids if not ids: - return {"removed": 0} + return {"ok": True, "removed": 0} db.execute( delete(SearchFind).where(SearchFind.user_id == user.id, SearchFind.video_id.in_(ids)) ) @@ -431,4 +431,4 @@ def clear_search( ) ).rowcount db.commit() - return {"removed": removed} + return {"ok": True, "removed": removed} diff --git a/backend/app/routes/setup.py b/backend/app/routes/setup.py index d9a6d996..6ce2c20b 100644 --- a/backend/app/routes/setup.py +++ b/backend/app/routes/setup.py @@ -106,7 +106,7 @@ def setup_config(settings_map: dict[str, Any], db: Session = Depends(require_set raise setting_refused(exc) from None saved.append(key) db.commit() # sysconfig.set_value only flushes now — persist the wizard's config in one commit - return {"saved": saved} + return {"ok": True, "saved": saved} class SetupTestEmailIn(BaseModel): diff --git a/backend/app/routes/sync.py b/backend/app/routes/sync.py index 6cdbadfa..baed9463 100644 --- a/backend/app/routes/sync.py +++ b/backend/app/routes/sync.py @@ -143,7 +143,7 @@ def deep_all( update(Subscription).where(Subscription.user_id == user.id).values(deep_requested=on) ) db.commit() - return {"updated": result.rowcount, "deep_requested": on} + return {"ok": True, "updated": result.rowcount, "deep_requested": on} @router.post("/pause") @@ -151,7 +151,7 @@ def pause_sync(admin: User = Depends(admin_user), db: Session = Depends(get_db)) state.set_sync_paused(db, True) audit.record(db, admin.id, AuditAction.SYNC_PAUSE, summary="Background sync paused") db.commit() - return {"paused": True} + return {"ok": True, "paused": True} @router.post("/resume") @@ -159,4 +159,4 @@ def resume_sync(admin: User = Depends(admin_user), db: Session = Depends(get_db) state.set_sync_paused(db, False) audit.record(db, admin.id, AuditAction.SYNC_RESUME, summary="Background sync resumed") db.commit() - return {"paused": False} + return {"ok": True, "paused": False} diff --git a/backend/app/routes/tags.py b/backend/app/routes/tags.py index cfd5b58f..7e2c23c9 100644 --- a/backend/app/routes/tags.py +++ b/backend/app/routes/tags.py @@ -76,7 +76,13 @@ def create_tag( except IntegrityError: db.rollback() raise refused(409, "tag_name_taken", name=name) - return {"id": tag.id, "name": tag.name, "color": tag.color, "category": tag.category} + return { + "ok": True, + "id": tag.id, + "name": tag.name, + "color": tag.color, + "category": tag.category, + } class TagUpdateIn(BaseModel): @@ -108,7 +114,13 @@ def update_tag( except IntegrityError: db.rollback() raise refused(409, "tag_name_taken", name=tag.name) - return {"id": tag.id, "name": tag.name, "color": tag.color, "category": tag.category} + return { + "ok": True, + "id": tag.id, + "name": tag.name, + "color": tag.color, + "category": tag.category, + } @router.delete("/{tag_id}") diff --git a/backend/tests/test_auth_session.py b/backend/tests/test_auth_session.py index 72fff9e6..040fd48f 100644 --- a/backend/tests/test_auth_session.py +++ b/backend/tests/test_auth_session.py @@ -447,7 +447,7 @@ def test_register_route_creates_the_account_end_to_end(client, db, monkeypatch): r = client.post("/auth/register", json={"email": "newbie@example.com", "password": _PW}) - assert r.status_code == 200 and r.json() == {"status": "ok"} + assert r.status_code == 200 and r.json() == {"ok": True, "status": "ok"} db.expire_all() user = db.execute(_select(User).where(User.email == "newbie@example.com")).scalar_one_or_none() assert user is not None, "POST /auth/register answered ok but created nothing" diff --git a/backend/tests/test_last_login.py b/backend/tests/test_last_login.py index 8b6b2973..02256d04 100644 --- a/backend/tests/test_last_login.py +++ b/backend/tests/test_last_login.py @@ -79,7 +79,7 @@ def test_the_demo_entry_does_not_stamp_the_shared_account(client, db): r = client.post("/auth/demo", json={"email": "visitor@example.com"}) - assert r.json() == {"authenticated": True} + assert r.json() == {"ok": True, "authenticated": True} demo = auth_routes.get_or_create_demo_user(db) db.refresh(demo) assert demo.last_login_at is None diff --git a/backend/tests/test_plex_finish.py b/backend/tests/test_plex_finish.py index 464e695a..43e890da 100644 --- a/backend/tests/test_plex_finish.py +++ b/backend/tests/test_plex_finish.py @@ -76,5 +76,5 @@ def test_the_finish_supersedes_a_resume_push_still_in_flight(db, as_user, make_u r = as_user(user).post( "/api/plex/item/m1/progress", json={"position_seconds": 2995, "duration_seconds": 3000} ) - assert r.json() == {"status": "watched", "position_seconds": 0} + assert r.json() == {"ok": True, "status": "watched", "position_seconds": 0} assert playback._resume_push_superseded(user.id, item.id, earlier) is True diff --git a/backend/tests/test_plex_query_conversion.py b/backend/tests/test_plex_query_conversion.py index a0300882..602fe810 100644 --- a/backend/tests/test_plex_query_conversion.py +++ b/backend/tests/test_plex_query_conversion.py @@ -436,7 +436,7 @@ class TestPlaylistAndCollectionEndpoints: {"item_rating_keys": ["m1", "m2", "m3"]}, ) assert added.status_code == 200, added.text - assert added.json() == {"added": 3, "item_count": 3} + assert added.json() == {"ok": True, "added": 3, "item_count": 3} removed = _request( db, @@ -448,7 +448,7 @@ class TestPlaylistAndCollectionEndpoints: {"item_rating_keys": ["m2"]}, ) assert removed.status_code == 200, removed.text - assert removed.json() == {"removed": 1, "item_count": 2} + assert removed.json() == {"ok": True, "removed": 1, "item_count": 2} detail = _get(db, user, monkeypatch, override_dep, f"/api/plex/playlists/{pid}").json() assert [it["id"] for it in detail["items"]] == [ diff --git a/backend/tests/test_route_conventions.py b/backend/tests/test_route_conventions.py new file mode 100644 index 00000000..b0b6b988 --- /dev/null +++ b/backend/tests/test_route_conventions.py @@ -0,0 +1,68 @@ +"""Two rows of CLAUDE.md's canonical-patterns table, held in code rather than prose. + +A rule stated only in a comment drifts: this one had — 63 mutation responses without `ok`, and +twelve route helpers that committed on their own, among them an approval whose decision, activation +and token landed in three separate transactions. The debt burn of 2026-09 fixed the drift; these keep +it fixed. +""" + +import ast +from pathlib import Path + +ROUTES = Path(__file__).parent.parent / "app" / "routes" +MUTATING = {"post", "put", "patch", "delete"} + +# A route helper that commits where it stands, and why the commit has to be there rather than in +# the handler. Anything else flushes and leaves the commit to its handler. +COMMITTING_HELPERS = { + "auth.py:_issue_token": "the token must be durable before the link that carries it is mailed", + "auth.py:_consume_token": "burning a single-use token is its own unit: a later failure must " + "not hand the token back", + "auth.py:_complete_link": "the tail of the Google callback route, which answers with its result", + "auth.py:_register_account": "a background task with its own session, i.e. a job wrapper", + "search.py:_ingest_candidates": "stages around YouTube calls: the stubs must survive an enrich " + "that fails", + "search.py:_classify_shorts": "the last stage of that ingest, after its probes", +} + + +def _modules(): + for path in sorted(ROUTES.rglob("*.py")): + yield path.relative_to(ROUTES).as_posix(), ast.parse(path.read_text(encoding="utf-8")) + + +def _is_mutation_route(fn: ast.FunctionDef) -> bool: + return any( + isinstance(d, ast.Call) and getattr(d.func, "attr", "") in MUTATING + for d in fn.decorator_list + ) + + +def test_every_mutation_answers_ok(): + """`{"ok": true, …}` (or `{"deleted": id}`) for every mutation that answers with a literal — + one field a client can read the same way everywhere. A serializer's return is exempt: it + describes a resource, not an acknowledgement.""" + missing = [] + for name, tree in _modules(): + for fn in ast.walk(tree): + if not (isinstance(fn, ast.FunctionDef) and _is_mutation_route(fn)): + continue + for node in ast.walk(fn): + if isinstance(node, ast.Return) and isinstance(node.value, ast.Dict): + keys = {k.value for k in node.value.keys if isinstance(k, ast.Constant)} + if not keys & {"ok", "deleted"}: + missing.append(f"{name}:{node.lineno} {fn.name}") + assert missing == [] + + +def test_route_helpers_flush_and_leave_the_commit_to_the_handler(): + committing = set() + for name, tree in _modules(): + for fn in ast.walk(tree): + if isinstance(fn, ast.FunctionDef) and fn.name.startswith("_"): + if any( + isinstance(n, ast.Call) and getattr(n.func, "attr", "") == "commit" + for n in ast.walk(fn) + ): + committing.add(f"{name}:{fn.name}") + assert committing == set(COMMITTING_HELPERS) diff --git a/backend/tests/test_welcome_mail.py b/backend/tests/test_welcome_mail.py index 2984cb1e..4d92acfe 100644 --- a/backend/tests/test_welcome_mail.py +++ b/backend/tests/test_welcome_mail.py @@ -220,7 +220,7 @@ def test_registering_again_re_sends_the_link_instead_of_sending_nothing(db, clie # The old behaviour was to return in silence: same cheerful "check your inbox", no mail. That is # the most likely explanation for the 2026-09-03 confirmation mail that "never arrived" — none # was ever sent. The response stays uniform; only the dead end is gone. - assert r.status_code == 200 and r.json() == {"status": "ok"} + assert r.status_code == 200 and r.json() == {"ok": True, "status": "ok"} assert len(sent) == 2 assert sent[1][0][0] == "twice@example.com"