fix(anki): prevent media timing review hangs and invalid ranges

- Reject stale or invalid timing actions
- Fall back to original timing when the renderer stops responding

src/core/services/media-timing-preview.test.ts

@@ -22,6 +22,23 @@ describe('buildMediaTimingPreviewArgs', () => {
     assert.equal(args.at(-2), '--');
     assert.equal(args.at(-1), '/video/show.mkv');
   });
+
+  test('separates an option-like media path without adding optional audio arguments', () => {
+    const args = buildMediaTimingPreviewArgs('/tmp/review.sock', {
+      mediaPath: '--fullscreen',
+    });
+
+    assert.equal(args.at(-2), '--');
+    assert.equal(args.at(-1), '--fullscreen');
+    assert.equal(
+      args.some((arg) => arg.startsWith('--aid=')),
+      false,
+    );
+    assert.equal(
+      args.some((arg) => arg.startsWith('--volume=')),
+      false,
+    );
+  });
 });
 
 test('preview session handles socket errors after connecting', async () => {
@@ -45,6 +62,31 @@ test('preview session handles socket errors after connecting', async () => {
   session.dispose();
 });
 
+test('preview session keeps failed connection errors handled through destruction', async () => {
+  const socket = new EventEmitter() as EventEmitter & {
+    destroy: () => void;
+  };
+  socket.destroy = () => {
+    socket.emit('error', new Error('socket failed again while closing'));
+  };
+  const child = new EventEmitter() as EventEmitter & { kill: () => boolean };
+  child.kill = () => true;
+  const times = [0, 0, 0, 6_000];
+  const session = new MediaTimingPreviewSession({
+    platform: 'linux',
+    spawnProcess: () => child as never,
+    connectSocket: () => {
+      queueMicrotask(() => socket.emit('error', new Error('connection failed')));
+      return socket as never;
+    },
+    now: () => times.shift() ?? 6_000,
+    removeSocketFile: () => undefined,
+    createSocketPath: () => '/tmp/review.sock',
+  });
+
+  await assert.rejects(session.start({ mediaPath: '/video/show.mkv' }), /Timed out starting/);
+});
+
 test('preview session rejects a connection that finishes after disposal', async () => {
   const socket = new net.Socket();
   const child = new EventEmitter() as EventEmitter & { kill: () => boolean };
@@ -126,7 +168,12 @@ test('preview session bounds a connection attempt that never settles', async ()
     spawnProcess: () => child as never,
     connectSocket: () => {
       connectAttempts += 1;
-      return new net.Socket();
+      const socket = new net.Socket();
+      socket.destroy = (() => {
+        socket.emit('error', new Error('socket failed while timing out'));
+        return socket;
+      }) as typeof socket.destroy;
+      return socket;
     },
     now: () => {
       const current = nowMs;

src/core/services/media-timing-preview.ts

@@ -290,6 +290,7 @@ export class MediaTimingPreviewSession {
         settled = true;
         clearAttemptTimeout();
         socket.off('connect', onConnect);
+        socket.on('error', () => {});
         socket.destroy();
         reject(error);
       };
@@ -301,6 +302,7 @@ export class MediaTimingPreviewSession {
         timeout = null;
         socket.off('connect', onConnect);
         socket.off('error', onError);
+        socket.on('error', () => {});
         socket.destroy();
         reject(new Error('Timed out connecting to the hidden mpv preview player'));
       }, timeoutMs);

src/main/runtime/media-timing-review-open.ts

@@ -1,4 +1,4 @@
-import type { OverlayHostedModal } from '../../shared/ipc/contracts';
+import { IPC_CHANNELS, type OverlayHostedModal } from '../../shared/ipc/contracts';
 import type { MediaTimingReviewOpenPayload } from '../../types/anki';
 import { openOverlayHostedModal, retryOverlayModalOpen } from './overlay-hosted-modal-open';
 
@@ -30,7 +30,7 @@ export async function openMediaTimingReviewModal(
         'Media timing review did not acknowledge modal open; retrying the dedicated modal window.',
       sendOpen: () =>
         openOverlayHostedModal(deps, {
-          channel: 'media-timing-review:open',
+          channel: IPC_CHANNELS.event.mediaTimingReviewOpen,
           modal: MODAL,
           payload,
           preferModalWindow: true,

src/main/runtime/media-timing-review.test.ts

@@ -1,5 +1,6 @@
 import assert from 'node:assert/strict';
 import { describe, test } from 'node:test';
+import type { MediaTimingReviewOpenPayload } from '../../types/anki';
 import {
   buildMediaTimingReviewPayload,
   createMediaTimingReviewRuntime,
@@ -63,6 +64,54 @@ describe('buildMediaTimingReviewPayload', () => {
   });
 });
 
+async function startActiveMediaTimingReview(
+  options: {
+    maxMediaDuration?: number;
+    decisionTimeoutMs?: number;
+  } = {},
+) {
+  const previewCalls: Array<[number, number]> = [];
+  let publishPayload!: (payload: MediaTimingReviewOpenPayload) => void;
+  const openedPayload = new Promise<MediaTimingReviewOpenPayload>((resolve) => {
+    publishPayload = resolve;
+  });
+  const runtime = createMediaTimingReviewRuntime({
+    getMpvClient: () => ({
+      connected: true,
+      currentVideoPath: '/video/show.mkv',
+      requestProperty: async (name) => (name === 'duration' ? 100 : name === 'pause' ? true : null),
+      send: () => undefined,
+    }),
+    getCurrentMediaPath: () => '/video/show.mkv',
+    getMpvExecutablePath: () => 'mpv',
+    generateWaveform: async () => [],
+    decisionTimeoutMs: options.decisionTimeoutMs,
+    createPreviewSession: () => ({
+      start: async () => undefined,
+      play: async (startTime, endTime) => {
+        previewCalls.push([startTime, endTime]);
+      },
+      stop: async () => undefined,
+      dispose: () => undefined,
+    }),
+    openModal: async (payload) => {
+      publishPayload(payload);
+      return true;
+    },
+    showStatus: () => undefined,
+  });
+  const pendingDecision = runtime.requestReview({
+    kind: 'sentence',
+    text: '字幕',
+    startTime: 10,
+    endTime: 12,
+    audioPadding: 0,
+    maxMediaDuration: options.maxMediaDuration ?? 30,
+  });
+
+  return { runtime, payload: await openedPayload, pendingDecision, previewCalls };
+}
+
 test('media timing review pauses playback, resolves exact timing, and restores playing state', async () => {
   const commands: Array<Array<string | number>> = [];
   const previewCalls: Array<[number, number]> = [];
@@ -189,6 +238,50 @@ test('media timing review analyzes the visible range on the selected audio strea
   ]);
 });
 
+test('media timing review rejects stale and out-of-range actions before allowing discard', async () => {
+  const { runtime, payload, pendingDecision, previewCalls } = await startActiveMediaTimingReview({
+    maxMediaDuration: 3,
+  });
+
+  assert.deepEqual(
+    await runtime.previewRange({ reviewId: 'stale-review', startTime: 10, endTime: 12 }),
+    { ok: false, message: 'This timing review is no longer active.' },
+  );
+  assert.deepEqual(
+    runtime.resolveReview({
+      reviewId: 'stale-review',
+      decision: { action: 'confirm', startTime: 10, endTime: 12 },
+    }),
+    { ok: false, message: 'This timing review is no longer active.' },
+  );
+  assert.deepEqual(
+    runtime.resolveReview({
+      reviewId: payload.reviewId,
+      decision: { action: 'confirm', startTime: 10, endTime: 14 },
+    }),
+    { ok: false, message: 'The selected timing range is invalid.' },
+  );
+  assert.deepEqual(
+    runtime.resolveReview({
+      reviewId: payload.reviewId,
+      decision: { action: 'confirm', startTime: 99, endTime: 100.5 },
+    }),
+    { ok: false, message: 'The selected timing range is invalid.' },
+  );
+  assert.deepEqual(
+    runtime.resolveReview({ reviewId: payload.reviewId, decision: { action: 'discard' } }),
+    { ok: true },
+  );
+  assert.deepEqual(await pendingDecision, { action: 'discard' });
+  assert.deepEqual(previewCalls, []);
+});
+
+test('media timing review watchdog falls back when the renderer stops responding', async () => {
+  const { pendingDecision } = await startActiveMediaTimingReview({ decisionTimeoutMs: 0 });
+
+  assert.deepEqual(await pendingDecision, { action: 'use-original' });
+});
+
 test('media timing review does not resume playback when the prior state is unavailable', async () => {
   const commands: Array<Array<string | number>> = [];
   let runtime: ReturnType<typeof createMediaTimingReviewRuntime>;

src/main/runtime/media-timing-review.ts

@@ -12,6 +12,7 @@ import type {
 import type { SpeechWaveformOptions } from '../../core/services/media-timing-waveform';
 
 const INITIAL_TIMELINE_MARGIN_SECONDS = 2;
+const REVIEW_DECISION_TIMEOUT_MS = 5 * 60_000;
 
 interface ReviewMpvClient {
   connected: boolean;
@@ -49,6 +50,7 @@ export interface MediaTimingReviewRuntimeDeps {
   getMpvExecutablePath: () => string;
   createPreviewSession: () => PreviewSession;
   generateWaveform: (options: SpeechWaveformOptions) => Promise<number[]>;
+  decisionTimeoutMs?: number;
   openModal: (payload: MediaTimingReviewOpenPayload) => Promise<boolean>;
   showStatus: (message: string) => void;
 }
@@ -64,6 +66,21 @@ function booleanProperty(value: unknown): boolean | null {
   return null;
 }
 
+function isValidMediaTimingRange(
+  payload: MediaTimingReviewOpenPayload,
+  startTime: number,
+  endTime: number,
+): boolean {
+  return (
+    Number.isFinite(startTime) &&
+    Number.isFinite(endTime) &&
+    startTime >= 0 &&
+    endTime > startTime &&
+    (payload.maxMediaDuration <= 0 || endTime - startTime <= payload.maxMediaDuration + 0.001) &&
+    (payload.mediaDuration === undefined || endTime <= payload.mediaDuration + 0.001)
+  );
+}
+
 export function buildMediaTimingReviewPayload(
   request: MediaTimingReviewRequest,
   options: { reviewId: string; mediaDuration?: number },
@@ -178,7 +195,16 @@ export function createMediaTimingReviewRuntime(deps: MediaTimingReviewRuntimeDep
       return { action: 'use-original' };
     }
 
-    const decision = await decisionPromise;
+    const decisionWatchdog = setTimeout(
+      () => resolveDecision({ action: 'use-original' }),
+      Math.max(0, deps.decisionTimeoutMs ?? REVIEW_DECISION_TIMEOUT_MS),
+    );
+    let decision: MediaTimingReviewDecision;
+    try {
+      decision = await decisionPromise;
+    } finally {
+      clearTimeout(decisionWatchdog);
+    }
     await cleanupActiveReview();
     return decision;
   }
@@ -210,16 +236,7 @@ export function createMediaTimingReviewRuntime(deps: MediaTimingReviewRuntimeDep
     if (!current || request.reviewId !== current.payload.reviewId) {
       return { ok: false, message: 'This timing review is no longer active.' };
     }
-    if (
+    if (!isValidMediaTimingRange(current.payload, request.startTime, request.endTime)) {
-      !Number.isFinite(request.startTime) ||
-      !Number.isFinite(request.endTime) ||
-      request.startTime < 0 ||
-      request.endTime <= request.startTime ||
-      (current.payload.maxMediaDuration > 0 &&
-        request.endTime - request.startTime > current.payload.maxMediaDuration + 0.001) ||
-      (current.payload.mediaDuration !== undefined &&
-        request.endTime > current.payload.mediaDuration + 0.001)
-    ) {
       return { ok: false, message: 'The selected preview range is invalid.' };
     }
     try {
@@ -294,16 +311,7 @@ export function createMediaTimingReviewRuntime(deps: MediaTimingReviewRuntimeDep
     }
     if (request.decision.action === 'confirm') {
       const { startTime, endTime } = request.decision;
-      if (
+      if (!isValidMediaTimingRange(current.payload, startTime, endTime)) {
-        !Number.isFinite(startTime) ||
-        !Number.isFinite(endTime) ||
-        startTime < 0 ||
-        endTime <= startTime ||
-        (current.payload.maxMediaDuration > 0 &&
-          endTime - startTime > current.payload.maxMediaDuration + 0.001) ||
-        (current.payload.mediaDuration !== undefined &&
-          endTime > current.payload.mediaDuration + 0.001)
-      ) {
         return { ok: false, message: 'The selected timing range is invalid.' };
       }
     }

src/preload.ts

@@ -69,6 +69,7 @@ import type {
   OverlayNotificationEventPayload,
   OverlayNotificationPosition,
   ChangelogSnapshot,
+  MediaTimingReviewActionResult,
   MediaTimingReviewOpenPayload,
   MediaTimingReviewPreviewRequest,
   MediaTimingReviewResolveRequest,
@@ -468,13 +469,17 @@ const electronAPI: ElectronAPI = {
   onOpenTsukihime: onOpenTsukihimeEvent,
   onOpenYoutubeTrackPicker: onOpenYoutubeTrackPickerEvent,
   onOpenMediaTimingReview: onOpenMediaTimingReviewEvent,
-  previewMediaTimingReview: (request: MediaTimingReviewPreviewRequest) =>
+  previewMediaTimingReview: (
+    request: MediaTimingReviewPreviewRequest,
+  ): Promise<MediaTimingReviewActionResult> =>
     ipcRenderer.invoke(IPC_CHANNELS.request.mediaTimingReviewPreview, request),
   getMediaTimingReviewWaveform: (request: MediaTimingReviewWaveformRequest) =>
     ipcRenderer.invoke(IPC_CHANNELS.request.mediaTimingReviewWaveform, request),
-  stopMediaTimingReviewPreview: (reviewId: string) =>
+  stopMediaTimingReviewPreview: (reviewId: string): Promise<MediaTimingReviewActionResult> =>
     ipcRenderer.invoke(IPC_CHANNELS.request.mediaTimingReviewStopPreview, reviewId),
-  resolveMediaTimingReview: (request: MediaTimingReviewResolveRequest) =>
+  resolveMediaTimingReview: (
+    request: MediaTimingReviewResolveRequest,
+  ): Promise<MediaTimingReviewActionResult> =>
     ipcRenderer.invoke(IPC_CHANNELS.request.mediaTimingReviewResolve, request),
   onOpenPlaylistBrowser: onOpenPlaylistBrowserEvent,
   onOpenCharacterDictionaryManager: onOpenCharacterDictionaryManagerEvent,

src/renderer/handlers/keyboard.test.ts

@@ -494,6 +494,7 @@ function createKeyboardHandlerHarness() {
     handleKikuKeydown: () => false,
     handleJimakuKeydown: () => false,
     handleTsukihimeKeydown: () => false,
+    handleMediaTimingReviewKeydown: () => false,
     handleControllerSelectKeydown: () => {
       controllerSelectKeydownCount += 1;
       return true;

src/renderer/handlers/keyboard.ts

@@ -18,7 +18,7 @@ export function createKeyboardHandlers(
     handleJimakuKeydown: (e: KeyboardEvent) => boolean;
     handleTsukihimeKeydown: (e: KeyboardEvent) => boolean;
     handleYoutubePickerKeydown: (e: KeyboardEvent) => boolean;
-    handleMediaTimingReviewKeydown?: (e: KeyboardEvent) => boolean;
+    handleMediaTimingReviewKeydown: (e: KeyboardEvent) => boolean;
     handlePlaylistBrowserKeydown: (e: KeyboardEvent) => boolean;
     handleControllerSelectKeydown: (e: KeyboardEvent) => boolean;
     handleControllerDebugKeydown: (e: KeyboardEvent) => boolean;
@@ -1080,7 +1080,7 @@ export function createKeyboardHandlers(
 
     document.addEventListener('keydown', (e: KeyboardEvent) => {
       if (ctx.state.mediaTimingReviewModalOpen) {
-        options.handleMediaTimingReviewKeydown?.(e);
+        options.handleMediaTimingReviewKeydown(e);
         return;
       }