fix: restore macos overlay passthrough

backlog/tasks/task-255 - Add-overlay-playlist-browser-modal-for-sibling-video-files-and-mpv-queue.md

@@ -5,7 +5,7 @@ status: In Progress
 assignee:
   - codex
 created_date: '2026-03-30 05:46'
-updated_date: '2026-03-30 08:34'
+updated_date: '2026-03-30 09:22'
 labels:
   - feature
   - overlay
@@ -40,6 +40,8 @@ Add an in-session overlay modal that opens from a keybinding during active playb
 5. Write failing renderer and keyboard tests for modal open/close, split-pane interaction, keyboard controls, and degraded states.
 6. Implement playlist-browser modal markup, DOM/state, renderer composition, keyboard routing, and session-help labeling.
 7. Run targeted test lanes first, then the maintained verification gate relevant to the touched surfaces; update task notes/criteria as checks pass.
+
+2026-03-30 CodeRabbit follow-up: 1) add failing runtime coverage for unreadable playlist-browser file stat failures, 2) add failing renderer coverage for stale snapshot UI reset on refresh failure/close, 3) add failing renderer coverage to block playlist-browser open when another modal already owns the overlay, 4) implement minimal fixes, 5) rerun targeted tests plus typecheck for touched surfaces.
 <!-- SECTION:PLAN:END -->
 
 ## Implementation Notes
@@ -58,4 +60,8 @@ Repo gate blockers outside this feature: `bun run test:fast` hits existing Bun `
 2026-03-30: Follow-up subtitle regression fix. Pre-jump `sid=auto` was ineffective because mpv resolved it against the current episode before `playlist-play-index`. Local playlist jumps now set `sub-auto=fuzzy`, switch episodes, then schedule a delayed rearm of `sid=auto` and `secondary-sid=auto` so selection happens against the new file's tracks. Added failing-first runtime coverage for delayed local rearm and remote no-op behavior.
 
 2026-03-30: Cleaned up playlist-browser runtime local-play subtitle-rearm flow by extracting focused helpers without changing behavior. Added public docs/readme coverage for the default `Ctrl+Alt+P` playlist browser keybinding and modal, plus changelog fragment `changes/260-playlist-browser.md`. Verification: `bun test src/main/runtime/playlist-browser-runtime.test.ts`, `bun run typecheck`, `bun run docs:test`, `bun run docs:build`, `bun run changelog:lint`, `bun run build`.
+
+2026-03-30: Pulled unresolved CodeRabbit review threads for PR #37. Actionable set is three items: unreadable-file stat error handling in playlist-browser runtime, stale playlist-browser DOM after failed refresh/close, and missing modal-ownership guard before opening the playlist-browser overlay. Proceeding test-first for each.
+
+2026-03-30: Addressed current CodeRabbit follow-up findings for PR #37. Fixed playlist-browser unreadable-file stat handling, stale playlist-browser DOM reset on refresh failure/close, modal-ownership guard before opening the playlist-browser overlay, async rejection surfacing for PLAYLIST_BROWSER_OPEN IPC commands, overlay bootstrap before playlist-browser open dispatch, texthooker option normalization in the mpv plugin, and superseded local subtitle-rearm suppression. Added targeted regressions plus new playlist-browser-open helper coverage. Verification: `bun test src/main/runtime/playlist-browser-runtime.test.ts src/main/runtime/playlist-browser-open.test.ts src/core/services/ipc-command.test.ts src/renderer/modals/playlist-browser.test.ts`, `lua scripts/test-plugin-start-gate.lua`, `bun run typecheck`, `bun run build`.
 <!-- SECTION:NOTES:END -->

backlog/tasks/task-260 - Fix-macOS-overlay-subtitle-sidebar-passthrough-without-requiring-a-subtitle-hover-cycle.md

@@ -0,0 +1,67 @@
+---
+id: TASK-260
+title: >-
+  Fix macOS overlay subtitle sidebar passthrough without requiring a subtitle
+  hover cycle
+status: Done
+assignee:
+  - '@codex'
+created_date: '2026-03-31 00:58'
+updated_date: '2026-03-31 01:01'
+labels:
+  - bug
+  - macos
+  - overlay
+  - subtitle-sidebar
+  - passthrough
+dependencies: []
+references:
+  - >-
+    /Users/sudacode/projects/japanese/SubMiner/src/renderer/modals/subtitle-sidebar.ts
+  - >-
+    /Users/sudacode/projects/japanese/SubMiner/src/renderer/overlay-mouse-ignore.ts
+  - /Users/sudacode/projects/japanese/SubMiner/src/renderer/handlers/mouse.ts
+  - /Users/sudacode/projects/japanese/SubMiner/src/main/overlay-runtime.ts
+  - >-
+    /Users/sudacode/projects/japanese/SubMiner/src/core/services/overlay-visibility.ts
+documentation:
+  - docs/workflow/verification.md
+priority: high
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+On macOS, opening the overlay-layout subtitle sidebar should allow click-through outside the sidebar immediately. Users should not need to first hover subtitle content before passthrough/click-through starts working, including when no subtitle line is currently visible.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 With the overlay-layout subtitle sidebar open on macOS, areas outside the sidebar pass clicks through immediately after open without requiring a prior subtitle hover.
+- [x] #2 When no subtitle line is currently visible, opening the subtitle sidebar still leaves non-sidebar overlay regions click-through on macOS.
+- [x] #3 Regression coverage exercises the first-open/idle passthrough path so overlay interactivity does not depend on a later hover cycle.
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Add/adjust focused overlay visibility regressions for the tracked macOS visible overlay so the default idle state stays click-through instead of forcing mouse interaction.
+2. Update main-process visible overlay visibility sync to keep the tracked macOS overlay passive by default and let renderer hover/sidebar state opt into interaction.
+3. Run focused verification for overlay visibility and any dependent runtime tests, then update task notes/criteria/final summary with the confirmed outcome.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Investigation points to a main-process override on macOS: renderer sidebar open path already requests mouse passthrough outside the panel, but visible-overlay visibility sync still hard-sets the tracked overlay window interactive on macOS (`mouse-ignore:false`). Window-tracker focus/visibility resync can therefore undo renderer passthrough until a later hover cycle re-applies it.
+
+Added a failing regression in `src/core/services/overlay-visibility.test.ts` showing the tracked macOS visible overlay was still forced interactive by main-process visibility sync (`mouse-ignore:false`) instead of staying forwarded click-through.
+
+Updated `src/core/services/overlay-visibility.ts` so tracked macOS visible overlays now default to `setIgnoreMouseEvents(true, { forward: true })`, matching the renderer-side passthrough model and preventing window-tracker/focus resync from undoing idle sidebar clickthrough.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Fixed the macOS subtitle-sidebar passthrough regression by changing tracked visible-overlay startup/visibility sync to stay click-through by default in the main process. Previously `updateVisibleOverlayVisibility` forced the macOS overlay window interactive, which could override renderer sidebar passthrough until a later hover cycle repaired it. Added a regression in `src/core/services/overlay-visibility.test.ts` and verified with `bun test src/core/services/overlay-visibility.test.ts`, `bun test src/renderer/modals/subtitle-sidebar.test.ts src/renderer/handlers/mouse.test.ts`, and `bun run typecheck`.
+<!-- SECTION:FINAL_SUMMARY:END -->

changes/261-macos-overlay-passthrough.md

@@ -0,0 +1,5 @@
+type: fixed
+area: overlay
+
+- Keep tracked macOS visible overlays click-through by default so subtitle sidebar passthrough works immediately without requiring a subtitle hover cycle first.
+- Add regression coverage for the macOS visible-overlay passthrough default.

plugin/subminer/process.lua

@@ -34,6 +34,17 @@ function M.create(ctx)
 		return options_helper.coerce_bool(raw_pause_until_ready, false)
 	end
 
+	local function resolve_texthooker_enabled(override_value)
+		if override_value ~= nil then
+			return options_helper.coerce_bool(override_value, false)
+		end
+		local raw_texthooker_enabled = opts.texthooker_enabled
+		if raw_texthooker_enabled == nil then
+			raw_texthooker_enabled = opts["texthooker-enabled"]
+		end
+		return options_helper.coerce_bool(raw_texthooker_enabled, false)
+	end
+
 	local function resolve_pause_until_ready_timeout_seconds()
 		local raw_timeout_seconds = opts.auto_start_pause_until_ready_timeout_seconds
 		if raw_timeout_seconds == nil then
@@ -192,10 +203,7 @@ function M.create(ctx)
 				table.insert(args, "--hide-visible-overlay")
 			end
 
-			local texthooker_enabled = overrides.texthooker_enabled
-			if texthooker_enabled == nil then
-				texthooker_enabled = opts.texthooker_enabled
-			end
+			local texthooker_enabled = resolve_texthooker_enabled(overrides.texthooker_enabled)
 			if texthooker_enabled then
 				table.insert(args, "--texthooker")
 			end
@@ -296,10 +304,7 @@ function M.create(ctx)
 			return
 		end
 
-		local texthooker_enabled = overrides.texthooker_enabled
-		if texthooker_enabled == nil then
-			texthooker_enabled = opts.texthooker_enabled
-		end
+		local texthooker_enabled = resolve_texthooker_enabled(overrides.texthooker_enabled)
 		local socket_path = overrides.socket_path or opts.socket_path
 		local should_pause_until_ready = (
 			overrides.auto_start_trigger == true
@@ -498,7 +503,7 @@ function M.create(ctx)
 				end
 			end)
 
-			if opts.texthooker_enabled then
+			if resolve_texthooker_enabled(nil) then
 				ensure_texthooker_running(function() end)
 			end
 		end)

scripts/test-plugin-start-gate.lua

@@ -531,6 +531,31 @@ do
 	)
 end
 
+do
+	local recorded, err = run_plugin_scenario({
+		process_list = "",
+		option_overrides = {
+			binary_path = binary_path,
+			auto_start = "yes",
+			auto_start_visible_overlay = "yes",
+			auto_start_pause_until_ready = "no",
+			socket_path = "/tmp/subminer-socket",
+			texthooker_enabled = "no",
+		},
+		input_ipc_server = "/tmp/subminer-socket",
+		media_title = "Random Movie",
+		files = {
+			[binary_path] = true,
+		},
+	})
+	assert_true(recorded ~= nil, "plugin failed to load for disabled texthooker auto-start scenario: " .. tostring(err))
+	fire_event(recorded, "file-loaded")
+	local start_call = find_start_call(recorded.async_calls)
+	assert_true(start_call ~= nil, "disabled texthooker auto-start should still issue --start command")
+	assert_true(not call_has_arg(start_call, "--texthooker"), "disabled texthooker should not include --texthooker on --start")
+	assert_true(find_control_call(recorded.async_calls, "--texthooker") == nil, "disabled texthooker should not issue a helper texthooker command")
+end
+
 do
 	local recorded, err = run_plugin_scenario({
 		process_list = "",

src/core/services/ipc-command.test.ts

@@ -114,14 +114,28 @@ test('handleMpvCommandFromIpc dispatches special youtube picker open command', (
   assert.deepEqual(osd, []);
 });
 
-test('handleMpvCommandFromIpc dispatches special playlist browser open command', () => {
+test('handleMpvCommandFromIpc dispatches special playlist browser open command', async () => {
   const { options, calls, sentCommands, osd } = createOptions();
   handleMpvCommandFromIpc(['__playlist-browser-open'], options);
+  await new Promise((resolve) => setImmediate(resolve));
   assert.deepEqual(calls, ['playlist-browser']);
   assert.deepEqual(sentCommands, []);
   assert.deepEqual(osd, []);
 });
 
+test('handleMpvCommandFromIpc surfaces playlist browser open rejections via mpv osd', async () => {
+  const { options, osd } = createOptions({
+    openPlaylistBrowser: async () => {
+      throw new Error('overlay failed');
+    },
+  });
+
+  handleMpvCommandFromIpc(['__playlist-browser-open'], options);
+  await new Promise((resolve) => setImmediate(resolve));
+
+  assert.deepEqual(osd, ['Playlist browser failed: overlay failed']);
+});
+
 test('handleMpvCommandFromIpc does not forward commands while disconnected', () => {
   const { options, sentCommands, osd } = createOptions({
     isMpvConnected: () => false,

src/core/services/ipc-command.ts

@@ -100,7 +100,12 @@ export function handleMpvCommandFromIpc(
   }
 
   if (first === options.specialCommands.PLAYLIST_BROWSER_OPEN) {
-    void options.openPlaylistBrowser();
+    Promise.resolve()
+      .then(() => options.openPlaylistBrowser())
+      .catch((error) => {
+        const message = error instanceof Error ? error.message : String(error);
+        options.showMpvOsd(`Playlist browser failed: ${message}`);
+      });
     return;
   }
 

src/core/services/overlay-visibility.test.ts

@@ -238,7 +238,7 @@ test('visible overlay stays hidden while a modal window is active', () => {
   assert.ok(!calls.includes('update-bounds'));
 });
 
-test('macOS tracked visible overlay stays visible without passively stealing focus', () => {
+test('macOS tracked visible overlay stays click-through without passively stealing focus', () => {
   const { window, calls } = createMainWindowRecorder();
   const tracker: WindowTrackerStub = {
     isTracking: () => true,
@@ -270,7 +270,7 @@ test('macOS tracked visible overlay stays visible without passively stealing foc
     isWindowsPlatform: false,
   } as never);
 
-  assert.ok(calls.includes('mouse-ignore:false:plain'));
+  assert.ok(calls.includes('mouse-ignore:true:forward'));
   assert.ok(calls.includes('show'));
   assert.ok(!calls.includes('focus'));
 });

src/core/services/overlay-visibility.ts

@@ -37,7 +37,7 @@ export function updateVisibleOverlayVisibility(args: {
 
   const showPassiveVisibleOverlay = (): void => {
     const forceMousePassthrough = args.forceMousePassthrough === true;
-    if (args.isWindowsPlatform || forceMousePassthrough) {
+    if (args.isMacOSPlatform || args.isWindowsPlatform || forceMousePassthrough) {
       mainWindow.setIgnoreMouseEvents(true, { forward: true });
     } else {
       mainWindow.setIgnoreMouseEvents(false);

src/main.ts

@@ -31,6 +31,7 @@ import {
   screen,
 } from 'electron';
 import { applyControllerConfigUpdate } from './main/controller-config-update.js';
+import { openPlaylistBrowser as openPlaylistBrowserRuntime } from './main/runtime/playlist-browser-open';
 import { createDiscordRpcClient } from './main/runtime/discord-rpc-client.js';
 import { mergeAiConfig } from './ai/config';
 
@@ -1941,8 +1942,12 @@ function openPlaylistBrowser(): void {
     showMpvOsd('Playlist browser requires active playback.');
     return;
   }
-  const opened = sendToActiveOverlayWindow(IPC_CHANNELS.event.playlistBrowserOpen, undefined, {
-    restoreOnModalClose: 'playlist-browser',
+  const opened = openPlaylistBrowserRuntime({
+    ensureOverlayStartupPrereqs: () => ensureOverlayStartupPrereqs(),
+    ensureOverlayWindowsReadyForVisibilityActions: () =>
+      ensureOverlayWindowsReadyForVisibilityActions(),
+    sendToActiveOverlayWindow: (channel, payload, runtimeOptions) =>
+      sendToActiveOverlayWindow(channel, payload, runtimeOptions),
   });
   if (!opened) {
     showMpvOsd('Playlist browser overlay unavailable.');

src/main/runtime/playlist-browser-open.test.ts

@@ -0,0 +1,28 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import { IPC_CHANNELS } from '../../shared/ipc/contracts';
+import { openPlaylistBrowser } from './playlist-browser-open';
+
+test('playlist browser open bootstraps overlay runtime before dispatching the modal event', () => {
+  const calls: string[] = [];
+
+  const opened = openPlaylistBrowser({
+    ensureOverlayStartupPrereqs: () => {
+      calls.push('prereqs');
+    },
+    ensureOverlayWindowsReadyForVisibilityActions: () => {
+      calls.push('windows');
+    },
+    sendToActiveOverlayWindow: (channel, payload, runtimeOptions) => {
+      calls.push(`send:${channel}`);
+      assert.equal(payload, undefined);
+      assert.deepEqual(runtimeOptions, {
+        restoreOnModalClose: 'playlist-browser',
+      });
+      return true;
+    },
+  });
+
+  assert.equal(opened, true);
+  assert.deepEqual(calls, ['prereqs', 'windows', `send:${IPC_CHANNELS.event.playlistBrowserOpen}`]);
+});

src/main/runtime/playlist-browser-open.ts

@@ -0,0 +1,23 @@
+import type { OverlayHostedModal } from '../../shared/ipc/contracts';
+import { IPC_CHANNELS } from '../../shared/ipc/contracts';
+
+const PLAYLIST_BROWSER_MODAL: OverlayHostedModal = 'playlist-browser';
+
+export function openPlaylistBrowser(deps: {
+  ensureOverlayStartupPrereqs: () => void;
+  ensureOverlayWindowsReadyForVisibilityActions: () => void;
+  sendToActiveOverlayWindow: (
+    channel: string,
+    payload?: unknown,
+    runtimeOptions?: {
+      restoreOnModalClose?: OverlayHostedModal;
+      preferModalWindow?: boolean;
+    },
+  ) => boolean;
+}): boolean {
+  deps.ensureOverlayStartupPrereqs();
+  deps.ensureOverlayWindowsReadyForVisibilityActions();
+  return deps.sendToActiveOverlayWindow(IPC_CHANNELS.event.playlistBrowserOpen, undefined, {
+    restoreOnModalClose: PLAYLIST_BROWSER_MODAL,
+  });
+}

src/main/runtime/playlist-browser-runtime.test.ts

@@ -249,6 +249,41 @@ test('playlist-browser mutation runtimes mutate queue and return refreshed snaps
   );
 });
 
+test('appendPlaylistBrowserFileRuntime returns an error result when statSync throws', async () => {
+  const dir = createTempVideoDir();
+  const episode1 = path.join(dir, 'Show - S01E01.mkv');
+  fs.writeFileSync(episode1, '');
+
+  const mutableFs = fs as typeof fs & { statSync: typeof fs.statSync };
+  const originalStatSync = mutableFs.statSync;
+  mutableFs.statSync = ((targetPath: fs.PathLike) => {
+    if (path.resolve(String(targetPath)) === episode1) {
+      throw new Error('EACCES');
+    }
+    return originalStatSync(targetPath);
+  }) as typeof fs.statSync;
+
+  try {
+    const result = await appendPlaylistBrowserFileRuntime(
+      {
+        getMpvClient: () =>
+          createFakeMpvClient({
+            currentVideoPath: episode1,
+            playlist: [{ filename: episode1, current: true }],
+          }),
+      },
+      episode1,
+    );
+
+    assert.deepEqual(result, {
+      ok: false,
+      message: 'Playlist browser file is not readable.',
+    });
+  } finally {
+    mutableFs.statSync = originalStatSync;
+  }
+});
+
 test('movePlaylistBrowserIndexRuntime rejects top and bottom boundary moves', async () => {
   const dir = createTempVideoDir();
   const episode1 = path.join(dir, 'Show - S01E01.mkv');
@@ -324,3 +359,52 @@ test('playPlaylistBrowserIndexRuntime skips local subtitle reset for remote play
   assert.deepEqual(mpvClient.getCommands().slice(-1), [['playlist-play-index', 1]]);
   assert.equal(scheduled.length, 0);
 });
+
+test('playPlaylistBrowserIndexRuntime ignores superseded local subtitle rearm callbacks', async () => {
+  const dir = createTempVideoDir();
+  const episode1 = path.join(dir, 'Show - S01E01.mkv');
+  const episode2 = path.join(dir, 'Show - S01E02.mkv');
+  const episode3 = path.join(dir, 'Show - S01E03.mkv');
+  fs.writeFileSync(episode1, '');
+  fs.writeFileSync(episode2, '');
+  fs.writeFileSync(episode3, '');
+
+  const scheduled: Array<() => void> = [];
+  const mpvClient = createFakeMpvClient({
+    currentVideoPath: episode1,
+    playlist: [
+      { filename: episode1, current: true, title: 'Episode 1' },
+      { filename: episode2, title: 'Episode 2' },
+      { filename: episode3, title: 'Episode 3' },
+    ],
+  });
+
+  const deps = {
+    getMpvClient: () => mpvClient,
+    schedule: (callback: () => void) => {
+      scheduled.push(callback);
+    },
+  };
+
+  const firstPlay = await playPlaylistBrowserIndexRuntime(deps, 1);
+  const secondPlay = await playPlaylistBrowserIndexRuntime(deps, 2);
+
+  assert.equal(firstPlay.ok, true);
+  assert.equal(secondPlay.ok, true);
+  assert.equal(scheduled.length, 2);
+
+  scheduled[0]?.();
+  scheduled[1]?.();
+
+  assert.deepEqual(
+    mpvClient.getCommands().slice(-6),
+    [
+      ['set_property', 'sub-auto', 'fuzzy'],
+      ['playlist-play-index', 1],
+      ['set_property', 'sub-auto', 'fuzzy'],
+      ['playlist-play-index', 2],
+      ['set_property', 'sid', 'auto'],
+      ['set_property', 'secondary-sid', 'auto'],
+    ],
+  );
+});

src/main/runtime/playlist-browser-runtime.ts

@@ -30,6 +30,8 @@ export type PlaylistBrowserRuntimeDeps = {
   schedule?: (callback: () => void, delayMs: number) => void;
 };
 
+const pendingLocalSubtitleSelectionRearms = new WeakMap<MpvPlaylistBrowserClientLike, number>();
+
 function trimToNull(value: unknown): string | null {
   if (typeof value !== 'string') return null;
   const trimmed = value.trim();
@@ -219,15 +221,26 @@ function prepareLocalSubtitleAutoload(client: MpvPlaylistBrowserClientLike): voi
   client.send({ command: ['set_property', 'sub-auto', 'fuzzy'] });
 }
 
-function isLocalPlaylistItem(item: PlaylistBrowserQueueItem | null | undefined): item is PlaylistBrowserQueueItem {
+function isLocalPlaylistItem(
+  item: PlaylistBrowserQueueItem | null | undefined,
+): item is PlaylistBrowserQueueItem & { path: string } {
   return Boolean(item?.path && !isRemoteMediaPath(item.path));
 }
 
 function scheduleLocalSubtitleSelectionRearm(
   deps: PlaylistBrowserRuntimeDeps,
   client: MpvPlaylistBrowserClientLike,
+  expectedPath: string,
 ): void {
+  const nextToken = (pendingLocalSubtitleSelectionRearms.get(client) ?? 0) + 1;
+  pendingLocalSubtitleSelectionRearms.set(client, nextToken);
   (deps.schedule ?? setTimeout)(() => {
+    if (pendingLocalSubtitleSelectionRearms.get(client) !== nextToken) return;
+    pendingLocalSubtitleSelectionRearms.delete(client);
+    const currentPath = trimToNull(client.currentVideoPath);
+    if (currentPath && path.resolve(currentPath) !== expectedPath) {
+      return;
+    }
     rearmLocalSubtitleSelection(client);
   }, 400);
 }
@@ -241,7 +254,16 @@ export async function appendPlaylistBrowserFileRuntime(
     return client;
   }
   const resolvedPath = path.resolve(filePath);
-  if (!fs.existsSync(resolvedPath) || !fs.statSync(resolvedPath).isFile()) {
+  let stats: fs.Stats;
+  try {
+    stats = fs.statSync(resolvedPath);
+  } catch {
+    return {
+      ok: false,
+      message: 'Playlist browser file is not readable.',
+    };
+  }
+  if (!stats.isFile()) {
     return {
       ok: false,
       message: 'Playlist browser file is not readable.',
@@ -267,7 +289,7 @@ export async function playPlaylistBrowserIndexRuntime(
   }
   result.client.send({ command: ['playlist-play-index', index] });
   if (isLocalPlaylistItem(targetItem)) {
-    scheduleLocalSubtitleSelectionRearm(deps, result.client);
+    scheduleLocalSubtitleSelectionRearm(deps, result.client, path.resolve(targetItem.path));
   }
   return buildMutationResult(`Playing playlist item ${index + 1}`, deps);
 }

src/renderer/modals/playlist-browser.test.ts

@@ -428,3 +428,241 @@ test('playlist browser keeps modal open when playing selected queue item fails',
     Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument });
   }
 });
+
+test('playlist browser refresh failure clears stale rendered rows and reports the error', async () => {
+  const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown };
+  const previousWindow = globals.window;
+  const previousDocument = globals.document;
+  const notifications: string[] = [];
+  let refreshShouldFail = false;
+
+  Object.defineProperty(globalThis, 'window', {
+    configurable: true,
+    value: {
+      electronAPI: {
+        getPlaylistBrowserSnapshot: async () => {
+          if (refreshShouldFail) {
+            throw new Error('snapshot failed');
+          }
+          return createSnapshot();
+        },
+        notifyOverlayModalOpened: (modal: string) => notifications.push(`open:${modal}`),
+        notifyOverlayModalClosed: (modal: string) => notifications.push(`close:${modal}`),
+        focusMainWindow: async () => {},
+        setIgnoreMouseEvents: () => {},
+        appendPlaylistBrowserFile: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+        playPlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+        removePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+        movePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+      } as unknown as ElectronAPI,
+      focus: () => {},
+    },
+  });
+  Object.defineProperty(globalThis, 'document', {
+    configurable: true,
+    value: {
+      createElement: () => createPlaylistRow(),
+    },
+  });
+
+  try {
+    const state = createRendererState();
+    const playlistBrowserTitle = createFakeElement();
+    const playlistBrowserStatus = createFakeElement();
+    const directoryList = createListStub();
+    const playlistList = createListStub();
+    const ctx = {
+      state,
+      platform: {
+        shouldToggleMouseIgnore: false,
+      },
+      dom: {
+        overlay: {
+          classList: createClassList(),
+          focus: () => {},
+        },
+        playlistBrowserModal: createFakeElement(),
+        playlistBrowserTitle,
+        playlistBrowserStatus,
+        playlistBrowserDirectoryList: directoryList,
+        playlistBrowserPlaylistList: playlistList,
+        playlistBrowserClose: createFakeElement(),
+      },
+    };
+
+    const modal = createPlaylistBrowserModal(ctx as never, {
+      modalStateReader: { isAnyModalOpen: () => false },
+      syncSettingsModalSubtitleSuppression: () => {},
+    });
+
+    await modal.openPlaylistBrowserModal();
+    assert.equal(directoryList.children.length, 2);
+    assert.equal(playlistList.children.length, 2);
+
+    refreshShouldFail = true;
+    await modal.refreshSnapshot();
+
+    assert.equal(state.playlistBrowserSnapshot, null);
+    assert.equal(directoryList.children.length, 0);
+    assert.equal(playlistList.children.length, 0);
+    assert.equal(playlistBrowserTitle.textContent, 'Playlist Browser');
+    assert.equal(playlistBrowserStatus.textContent, 'snapshot failed');
+    assert.equal(playlistBrowserStatus.classList.contains('error'), true);
+    assert.deepEqual(notifications, ['open:playlist-browser']);
+  } finally {
+    Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow });
+    Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument });
+  }
+});
+
+test('playlist browser close clears rendered snapshot ui', async () => {
+  const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown };
+  const previousWindow = globals.window;
+  const previousDocument = globals.document;
+  const notifications: string[] = [];
+
+  Object.defineProperty(globalThis, 'window', {
+    configurable: true,
+    value: {
+      electronAPI: {
+        getPlaylistBrowserSnapshot: async () => createSnapshot(),
+        notifyOverlayModalOpened: (modal: string) => notifications.push(`open:${modal}`),
+        notifyOverlayModalClosed: (modal: string) => notifications.push(`close:${modal}`),
+        focusMainWindow: async () => {},
+        setIgnoreMouseEvents: () => {},
+        appendPlaylistBrowserFile: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+        playPlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+        removePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+        movePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+      } as unknown as ElectronAPI,
+      focus: () => {},
+    },
+  });
+  Object.defineProperty(globalThis, 'document', {
+    configurable: true,
+    value: {
+      createElement: () => createPlaylistRow(),
+    },
+  });
+
+  try {
+    const state = createRendererState();
+    const playlistBrowserTitle = createFakeElement();
+    const playlistBrowserStatus = createFakeElement();
+    const directoryList = createListStub();
+    const playlistList = createListStub();
+    const ctx = {
+      state,
+      platform: {
+        shouldToggleMouseIgnore: false,
+      },
+      dom: {
+        overlay: {
+          classList: createClassList(),
+          focus: () => {},
+        },
+        playlistBrowserModal: createFakeElement(),
+        playlistBrowserTitle,
+        playlistBrowserStatus,
+        playlistBrowserDirectoryList: directoryList,
+        playlistBrowserPlaylistList: playlistList,
+        playlistBrowserClose: createFakeElement(),
+      },
+    };
+
+    const modal = createPlaylistBrowserModal(ctx as never, {
+      modalStateReader: { isAnyModalOpen: () => false },
+      syncSettingsModalSubtitleSuppression: () => {},
+    });
+
+    await modal.openPlaylistBrowserModal();
+    assert.equal(directoryList.children.length, 2);
+    assert.equal(playlistList.children.length, 2);
+
+    modal.closePlaylistBrowserModal();
+
+    assert.equal(state.playlistBrowserSnapshot, null);
+    assert.equal(state.playlistBrowserStatus, '');
+    assert.equal(directoryList.children.length, 0);
+    assert.equal(playlistList.children.length, 0);
+    assert.equal(playlistBrowserTitle.textContent, 'Playlist Browser');
+    assert.equal(playlistBrowserStatus.textContent, '');
+    assert.deepEqual(notifications, ['open:playlist-browser', 'close:playlist-browser']);
+  } finally {
+    Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow });
+    Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument });
+  }
+});
+
+test('playlist browser open is ignored while another modal is already open', async () => {
+  const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown };
+  const previousWindow = globals.window;
+  const previousDocument = globals.document;
+  const notifications: string[] = [];
+  let snapshotCalls = 0;
+
+  Object.defineProperty(globalThis, 'window', {
+    configurable: true,
+    value: {
+      electronAPI: {
+        getPlaylistBrowserSnapshot: async () => {
+          snapshotCalls += 1;
+          return createSnapshot();
+        },
+        notifyOverlayModalOpened: (modal: string) => notifications.push(`open:${modal}`),
+        notifyOverlayModalClosed: (modal: string) => notifications.push(`close:${modal}`),
+        focusMainWindow: async () => {},
+        setIgnoreMouseEvents: () => {},
+        appendPlaylistBrowserFile: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+        playPlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+        removePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+        movePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
+      } as unknown as ElectronAPI,
+      focus: () => {},
+    },
+  });
+  Object.defineProperty(globalThis, 'document', {
+    configurable: true,
+    value: {
+      createElement: () => createPlaylistRow(),
+    },
+  });
+
+  try {
+    const state = createRendererState();
+    const overlay = {
+      classList: createClassList(),
+      focus: () => {},
+    };
+    const ctx = {
+      state,
+      platform: {
+        shouldToggleMouseIgnore: false,
+      },
+      dom: {
+        overlay,
+        playlistBrowserModal: createFakeElement(),
+        playlistBrowserTitle: createFakeElement(),
+        playlistBrowserStatus: createFakeElement(),
+        playlistBrowserDirectoryList: createListStub(),
+        playlistBrowserPlaylistList: createListStub(),
+        playlistBrowserClose: createFakeElement(),
+      },
+    };
+
+    const modal = createPlaylistBrowserModal(ctx as never, {
+      modalStateReader: { isAnyModalOpen: () => true },
+      syncSettingsModalSubtitleSuppression: () => {},
+    });
+
+    await modal.openPlaylistBrowserModal();
+
+    assert.equal(state.playlistBrowserModalOpen, false);
+    assert.equal(snapshotCalls, 0);
+    assert.equal(overlay.classList.contains('interactive'), false);
+    assert.deepEqual(notifications, []);
+  } finally {
+    Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow });
+    Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument });
+  }
+});

src/renderer/modals/playlist-browser.ts

@@ -49,6 +49,18 @@ export function createPlaylistBrowserModal(
     return ctx.state.playlistBrowserSnapshot;
   }
 
+  function resetSnapshotUi(): void {
+    ctx.state.playlistBrowserSnapshot = null;
+    ctx.state.playlistBrowserStatus = '';
+    ctx.state.playlistBrowserSelectedDirectoryIndex = 0;
+    ctx.state.playlistBrowserSelectedPlaylistIndex = 0;
+    ctx.dom.playlistBrowserTitle.textContent = 'Playlist Browser';
+    ctx.dom.playlistBrowserDirectoryList.replaceChildren();
+    ctx.dom.playlistBrowserPlaylistList.replaceChildren();
+    ctx.dom.playlistBrowserStatus.textContent = '';
+    ctx.dom.playlistBrowserStatus.classList.remove('error');
+  }
+
   function syncSelection(snapshot: PlaylistBrowserSnapshot): void {
     const directoryIndex = snapshot.directoryItems.findIndex((item) => item.isCurrentFile);
     const playlistIndex =
@@ -194,13 +206,18 @@ export function createPlaylistBrowserModal(
   }
 
   async function refreshSnapshot(): Promise<void> {
-    const snapshot = await window.electronAPI.getPlaylistBrowserSnapshot();
-    ctx.state.playlistBrowserStatus = '';
-    applySnapshot(snapshot);
-    setStatus(
-      buildDefaultStatus(snapshot),
-      !snapshot.directoryAvailable && snapshot.directoryStatus.length > 0,
-    );
+    try {
+      const snapshot = await window.electronAPI.getPlaylistBrowserSnapshot();
+      ctx.state.playlistBrowserStatus = '';
+      applySnapshot(snapshot);
+      setStatus(
+        buildDefaultStatus(snapshot),
+        !snapshot.directoryAvailable && snapshot.directoryStatus.length > 0,
+      );
+    } catch (error) {
+      resetSnapshotUi();
+      setStatus(error instanceof Error ? error.message : String(error), true);
+    }
   }
 
   async function handleMutation(
@@ -249,6 +266,9 @@ export function createPlaylistBrowserModal(
       await refreshSnapshot();
       return;
     }
+    if (options.modalStateReader.isAnyModalOpen()) {
+      return;
+    }
 
     ctx.state.playlistBrowserModalOpen = true;
     ctx.state.playlistBrowserActivePane = 'playlist';
@@ -257,19 +277,13 @@ export function createPlaylistBrowserModal(
     ctx.dom.playlistBrowserModal.classList.remove('hidden');
     ctx.dom.playlistBrowserModal.setAttribute('aria-hidden', 'false');
     window.electronAPI.notifyOverlayModalOpened('playlist-browser');
-
-    try {
-      await refreshSnapshot();
-    } catch (error) {
-      setStatus(error instanceof Error ? error.message : String(error), true);
-    }
+    await refreshSnapshot();
   }
 
   function closePlaylistBrowserModal(): void {
     if (!ctx.state.playlistBrowserModalOpen) return;
     ctx.state.playlistBrowserModalOpen = false;
-    ctx.state.playlistBrowserSnapshot = null;
-    ctx.state.playlistBrowserStatus = '';
+    resetSnapshotUi();
     ctx.dom.playlistBrowserModal.classList.add('hidden');
     ctx.dom.playlistBrowserModal.setAttribute('aria-hidden', 'true');
     window.electronAPI.notifyOverlayModalClosed('playlist-browser');