From 98bb841a6aa7584f43861decb1e893d5f831ba05 Mon Sep 17 00:00:00 2001
From: npeter83 <npeter83+claude@gmail.com>
Date: Sun, 27 Sep 2026 14:51:37 +0200
Subject: [PATCH] fix(playlists): one push or revert at a time for a list,
 whichever menu started it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The in-flight flag was per hook instance, and the rail row's menu and the list page header's "…"
are two at once: a push from one left the other offering a second push. It is one app-wide store
(`lib/store`), read with `get()` at the start so a second start refuses before a render; a revert
re-reads it after its confirm dialog.
---
 .../usePlaylistActions.dom.test.tsx           | 26 +++++++++++++++++++
 .../src/components/usePlaylistActions.tsx     | 24 ++++++++++++-----
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/frontend/src/components/usePlaylistActions.dom.test.tsx b/frontend/src/components/usePlaylistActions.dom.test.tsx
index 28a01d23..957a15d1 100644
--- a/frontend/src/components/usePlaylistActions.dom.test.tsx
+++ b/frontend/src/components/usePlaylistActions.dom.test.tsx
@@ -155,3 +155,29 @@ describe("usePlaylistActions — a playlist's menu", () => {
     );
   });
 });
+
+// E28 review C1: the push / revert in-flight flag was per hook instance, and two instances run at
+// once (the rail row's menu and the list page header's "…") — a push started from one left the other
+// offering a second push of the same list.
+describe("usePlaylistActions — one push at a time, across menus", () => {
+  it("a push started from one menu is off in the other until it ends", async () => {
+    let finish: (v: unknown) => void = () => {};
+    vi.spyOn(api, "playlistPushPlan").mockReturnValue(
+      new Promise((r) => (finish = r)) as ReturnType<typeof api.playlistPushPlan>,
+    );
+    const rail = mount();
+    const header = mount();
+    const dirty = { ...linked, dirty: true };
+    const push = (m: typeof rail) =>
+      m.result.current.items(dirty, { onRename: () => {}, pin: [] }).find((i) => i.id === "push")!;
+    act(() => push(rail).onSelect!());
+    expect(push(header).disabled).toBe(true);
+    // A second start (a keyboard or a stale menu) is refused, not run beside the first.
+    await act(async () => push(header).onSelect!());
+    expect(api.playlistPushPlan).toHaveBeenCalledTimes(1);
+    await act(async () => {
+      finish({ action: "update", affordable: true });
+    });
+    expect(push(header).disabled).toBeFalsy();
+  });
+});
diff --git a/frontend/src/components/usePlaylistActions.tsx b/frontend/src/components/usePlaylistActions.tsx
index f5c45fdc..c9e4632f 100644
--- a/frontend/src/components/usePlaylistActions.tsx
+++ b/frontend/src/components/usePlaylistActions.tsx
@@ -1,4 +1,3 @@
-import { useState } from "react";
 import { useQueryClient } from "@tanstack/react-query";
 import { useTranslation } from "react-i18next";
 import { ExternalLink, Pencil, RotateCcw, Trash2, Upload, Youtube } from "lucide-react";
@@ -12,6 +11,7 @@ import { useChoice, useConfirm } from "./confirmContext";
 import { usePlaylists, usePlaylistsActions } from "./playlistsContext";
 import { useMe } from "../lib/useMe";
 import { type ActionMenuItem } from "./ui/ActionMenu";
+import { createStore } from "../lib/store";
 
 // A playlist's ONE action list (E28 C1) and the mutations behind it. Two entry points open it — the
 // rail row (right-click, long-press, its "…") and the list page header's "…" — so it lives here,
@@ -24,6 +24,15 @@ import { type ActionMenuItem } from "./ui/ActionMenu";
  *  and `PlaylistDetail` (the detail pane), which is why neither is named here. */
 type Menuable = Pick<Playlist, "id" | "name" | "kind" | "yt_playlist_id" | "dirty">;
 
+// Which list a push / revert is running for, so its row is off (and says so) until it is done. ONE
+// for the app, not per hook: the rail row's menu and the list page header's "…" are two instances at
+// once, and a per-instance flag left the other offering a second push of the same list (E28 review
+// C1). Read with `get()` at the start, so a second start refuses before any render has happened.
+const inFlight = createStore<{ pushing: number | null; reverting: number | null }>({
+  pushing: null,
+  reverting: null,
+});
+
 export function usePlaylistActions() {
   const qc = useQueryClient();
   const { t } = useTranslation();
@@ -32,9 +41,9 @@ export function usePlaylistActions() {
   const canWrite = useMe().can_write;
   const { selectedId } = usePlaylists();
   const { setSelectedId } = usePlaylistsActions();
-  // Which list a push / revert is running for, so its row is off (and says so) until it is done.
-  const [pushing, setPushing] = useState<number | null>(null);
-  const [reverting, setReverting] = useState<number | null>(null);
+  const { pushing, reverting } = inFlight.use();
+  const setPushing = (id: number | null) => inFlight.set((s) => ({ ...s, pushing: id }));
+  const setReverting = (id: number | null) => inFlight.set((s) => ({ ...s, reverting: id }));
 
   const refresh = (id: number) => {
     qc.invalidateQueries({ queryKey: qk.playlists() });
@@ -96,14 +105,15 @@ export function usePlaylistActions() {
   }
 
   async function revert(pl: Menuable) {
-    if (reverting != null) return;
+    if (inFlight.get().reverting != null) return;
     const ok = await confirm({
       title: t("playlists.revertTitle"),
       message: t("playlists.revertMsg"),
       confirmLabel: t("playlists.revertConfirm"),
       danger: true,
     });
-    if (!ok) return;
+    // Re-read after the dialog: another menu may have started one while this one asked.
+    if (!ok || inFlight.get().reverting != null) return;
     setReverting(pl.id);
     try {
       await api.revertPlaylist(pl.id);
@@ -120,7 +130,7 @@ export function usePlaylistActions() {
   }
 
   async function push(pl: Menuable) {
-    if (pushing != null) return;
+    if (inFlight.get().pushing != null) return;
     const name = playlistName(pl, t);
     setPushing(pl.id);
     try {