fix: address PR #31 follow-up review comments

src/main.ts

@@ -947,6 +947,7 @@ async function runYoutubePlaybackFlowMain(request: {
   source: CliCommandSource;
 }): Promise<void> {
   youtubePrimarySubtitleNotificationRuntime.setAppOwnedFlowInFlight(true);
+  try {
     if (process.platform === 'win32' && !appState.mpvClient?.connected) {
       const launchResult = launchWindowsMpv(
         [request.url],
@@ -969,8 +970,6 @@ async function runYoutubePlaybackFlowMain(request: {
     if (!appState.mpvClient?.connected) {
       appState.mpvClient?.connect();
     }
-
-  try {
     await youtubeFlowRuntime.runYoutubePlaybackFlow({
       url: request.url,
       mode: request.mode,
@@ -1263,6 +1262,10 @@ function reportYoutubeSubtitleFailure(message: string): void {
 }
 
 async function openYoutubeTrackPickerFromPlayback(): Promise<void> {
+  if (youtubeFlowRuntime.hasActiveSession()) {
+    showMpvOsd('YouTube subtitle flow already in progress.');
+    return;
+  }
   const currentMediaPath =
     appState.currentMediaPath?.trim() || appState.mpvClient?.currentVideoPath?.trim() || '';
   if (!isYoutubePlaybackActiveNow() || !currentMediaPath) {

src/main/runtime/youtube-primary-subtitle-notification.test.ts

@@ -82,6 +82,28 @@ test('notifier suppresses failure when preferred primary subtitle is selected',
   assert.deepEqual(notifications, []);
 });
 
+test('notifier suppresses failure when selected track is marked active before sid arrives', () => {
+  const notifications: string[] = [];
+  const timers = createTimerHarness();
+  const runtime = createYoutubePrimarySubtitleNotificationRuntime({
+    getPrimarySubtitleLanguages: () => ['ja', 'jpn'],
+    notifyFailure: (message) => {
+      notifications.push(message);
+    },
+    schedule: (fn) => timers.schedule(fn),
+    clearSchedule: (timer) => timers.clear(timer),
+  });
+
+  runtime.handleMediaPathChange('https://www.youtube.com/watch?v=abc');
+  runtime.handleSubtitleTrackChange(null);
+  runtime.handleSubtitleTrackListChange([
+    { type: 'sub', id: 5, lang: 'ja', title: 'Japanese', external: false, selected: true },
+  ]);
+  timers.runAll();
+
+  assert.deepEqual(notifications, []);
+});
+
 test('notifier suppresses failure when any external subtitle track is selected', () => {
   const notifications: string[] = [];
   const timers = createTimerHarness();

src/main/runtime/youtube-primary-subtitle-notification.ts

@@ -8,6 +8,7 @@ type SubtitleTrackEntry = {
   type: string;
   lang: string;
   external: boolean;
+  selected: boolean;
 };
 
 function parseTrackId(value: unknown): number | null {
@@ -31,6 +32,7 @@ function normalizeTrack(entry: unknown): SubtitleTrackEntry | null {
     type: String(track.type || '').trim(),
     lang: String(track.lang || '').trim(),
     external: track.external === true,
+    selected: track.selected === true,
   };
 }
 
@@ -74,12 +76,15 @@ function hasSelectedPrimarySubtitle(
   trackList: unknown[] | null,
   preferredLanguages: Set<string>,
 ): boolean {
-  if (sid === null || !Array.isArray(trackList)) {
+  if (!Array.isArray(trackList)) {
     return false;
   }
 
+  const tracks = trackList.map(normalizeTrack);
   const activeTrack =
-    trackList.map(normalizeTrack).find((track) => track?.type === 'sub' && track.id === sid) ?? null;
+    (sid === null ? null : tracks.find((track) => track?.type === 'sub' && track.id === sid) ?? null) ??
+    tracks.find((track) => track?.type === 'sub' && track.selected) ??
+    null;
   if (!activeTrack) {
     return false;
   }

src/renderer/modals/youtube-track-picker.test.ts

@@ -35,6 +35,9 @@ function createFakeElement() {
     append(...children: any[]) {
       this.children.push(...children);
     },
+    replaceChildren(...children: any[]) {
+      this.children = [...children];
+    },
     addEventListener(type: string, listener: (event?: any) => void) {
       const existing = this.listeners.get(type) ?? [];
       existing.push(listener);
@@ -763,3 +766,109 @@ test('youtube track picker ignores immediate Enter after open before allowing ke
     Object.defineProperty(globalThis, 'document', { configurable: true, value: originalDocument });
   }
 });
+
+test('youtube track picker uses track list as the source of truth for available selections', async () => {
+  const resolveCalls: Array<{
+    sessionId: string;
+    action: string;
+    primaryTrackId: string | null;
+    secondaryTrackId: string | null;
+  }> = [];
+  const originalWindow = globalThis.window;
+  const originalDocument = globalThis.document;
+
+  Object.defineProperty(globalThis, 'document', {
+    configurable: true,
+    value: {
+      createElement: () => createFakeElement(),
+    },
+  });
+
+  Object.defineProperty(globalThis, 'window', {
+    configurable: true,
+    value: {
+      dispatchEvent: () => true,
+      focus: () => {},
+      electronAPI: {
+        notifyOverlayModalOpened: () => {},
+        notifyOverlayModalClosed: () => {},
+        youtubePickerResolve: async (payload: {
+          sessionId: string;
+          action: string;
+          primaryTrackId: string | null;
+          secondaryTrackId: string | null;
+        }) => {
+          resolveCalls.push(payload);
+          return { ok: true, message: '' };
+        },
+        setIgnoreMouseEvents: () => {},
+      },
+    },
+  });
+
+  try {
+    const state = createRendererState();
+    const dom = {
+      overlay: {
+        classList: createClassList(),
+        focus: () => {},
+      },
+      youtubePickerModal: createFakeElement(),
+      youtubePickerTitle: createFakeElement(),
+      youtubePickerPrimarySelect: createFakeElement(),
+      youtubePickerSecondarySelect: createFakeElement(),
+      youtubePickerTracks: createFakeElement(),
+      youtubePickerStatus: createFakeElement(),
+      youtubePickerContinueButton: createFakeElement(),
+      youtubePickerCloseButton: createFakeElement(),
+    };
+
+    const modal = createYoutubeTrackPickerModal(
+      {
+        state,
+        dom,
+        platform: {
+          shouldToggleMouseIgnore: false,
+        },
+      } as never,
+      {
+        modalStateReader: { isAnyModalOpen: () => true },
+        restorePointerInteractionState: () => {},
+        syncSettingsModalSubtitleSuppression: () => {},
+      },
+    );
+
+    modal.openYoutubePickerModal({
+      sessionId: 'yt-1',
+      url: 'https://example.com',
+      tracks: [
+        {
+          id: 'manual:ja',
+          language: 'ja',
+          sourceLanguage: 'ja',
+          kind: 'manual',
+          label: 'Japanese',
+        },
+      ],
+      defaultPrimaryTrackId: 'manual:ja',
+      defaultSecondaryTrackId: null,
+      hasTracks: false,
+    });
+    modal.wireDomEvents();
+
+    const listeners = dom.youtubePickerContinueButton.listeners.get('click') ?? [];
+    await Promise.all(listeners.map((listener) => listener()));
+
+    assert.deepEqual(resolveCalls, [
+      {
+        sessionId: 'yt-1',
+        action: 'use-selected',
+        primaryTrackId: 'manual:ja',
+        secondaryTrackId: null,
+      },
+    ]);
+  } finally {
+    Object.defineProperty(globalThis, 'window', { configurable: true, value: originalWindow });
+    Object.defineProperty(globalThis, 'document', { configurable: true, value: originalDocument });
+  }
+});

src/renderer/modals/youtube-track-picker.ts

@@ -9,6 +9,10 @@ function createOption(value: string, label: string): HTMLOptionElement {
   return option;
 }
 
+function payloadHasTracks(payload: YoutubePickerOpenPayload | null): boolean {
+  return (payload?.tracks.length ?? 0) > 0;
+}
+
 export function createYoutubeTrackPickerModal(
   ctx: RendererContext,
   options: {
@@ -34,9 +38,9 @@ export function createYoutubeTrackPickerModal(
   }
 
   function renderTrackList(): void {
-    ctx.dom.youtubePickerTracks.innerHTML = '';
+    ctx.dom.youtubePickerTracks.replaceChildren();
     const payload = ctx.state.youtubePickerPayload;
-    if (!payload || payload.tracks.length === 0) {
+    if (!payload || !payloadHasTracks(payload)) {
       const li = document.createElement('li');
       const left = document.createElement('span');
       left.textContent = 'No subtitle tracks found';
@@ -70,7 +74,7 @@ export function createYoutubeTrackPickerModal(
   function syncSecondaryOptions(): void {
     const payload = ctx.state.youtubePickerPayload;
     const primaryTrackId = ctx.dom.youtubePickerPrimarySelect.value || null;
-    ctx.dom.youtubePickerSecondarySelect.innerHTML = '';
+    ctx.dom.youtubePickerSecondarySelect.replaceChildren();
     ctx.dom.youtubePickerSecondarySelect.appendChild(createOption('', 'None'));
     if (!payload) return;
 
@@ -97,10 +101,10 @@ export function createYoutubeTrackPickerModal(
   function applyPayload(payload: YoutubePickerOpenPayload): void {
     ctx.state.youtubePickerPayload = payload;
     ctx.dom.youtubePickerTitle.textContent = `Select YouTube subtitles for ${payload.url}`;
-    ctx.dom.youtubePickerPrimarySelect.innerHTML = '';
-    ctx.dom.youtubePickerSecondarySelect.innerHTML = '';
+    ctx.dom.youtubePickerPrimarySelect.replaceChildren();
+    ctx.dom.youtubePickerSecondarySelect.replaceChildren();
 
-    if (payload.tracks.length === 0) {
+    if (!payloadHasTracks(payload)) {
       ctx.dom.youtubePickerPrimarySelect.appendChild(createOption('', 'No tracks available'));
       ctx.dom.youtubePickerPrimarySelect.disabled = true;
       ctx.dom.youtubePickerSecondarySelect.disabled = true;
@@ -128,7 +132,8 @@ export function createYoutubeTrackPickerModal(
     }
     const payload = ctx.state.youtubePickerPayload;
     if (!payload) return;
-    if (action === 'use-selected' && payload.hasTracks && !ctx.dom.youtubePickerPrimarySelect.value) {
+    const hasTracks = payloadHasTracks(payload);
+    if (action === 'use-selected' && hasTracks && !ctx.dom.youtubePickerPrimarySelect.value) {
       setStatus('Primary subtitle selection is required.', true);
       return;
     }
@@ -160,7 +165,7 @@ export function createYoutubeTrackPickerModal(
     } finally {
       resolveSelectionInFlight = false;
       const shouldKeepDisabled =
-        ctx.state.youtubePickerModalOpen && !(ctx.state.youtubePickerPayload?.hasTracks ?? false);
+        ctx.state.youtubePickerModalOpen && !payloadHasTracks(ctx.state.youtubePickerPayload);
       setResolveControlsDisabled(shouldKeepDisabled);
     }
   }
@@ -233,7 +238,7 @@ export function createYoutubeTrackPickerModal(
         return true;
       }
       void resolveSelection(
-        ctx.state.youtubePickerPayload?.hasTracks ? 'use-selected' : 'continue-without-subtitles',
+        payloadHasTracks(ctx.state.youtubePickerPayload) ? 'use-selected' : 'continue-without-subtitles',
       );
       return true;
     }
@@ -264,7 +269,7 @@ export function createYoutubeTrackPickerModal(
 
     ctx.dom.youtubePickerContinueButton.addEventListener('click', () => {
       void resolveSelection(
-        ctx.state.youtubePickerPayload?.hasTracks ? 'use-selected' : 'continue-without-subtitles',
+        payloadHasTracks(ctx.state.youtubePickerPayload) ? 'use-selected' : 'continue-without-subtitles',
       );
     });