fix: address CodeRabbit review findings across runtime modules

- Extract filterLegacyMpvPluginFileCandidates, buildYomitanAnkiSettingsKey, setMpvCurrentSecondarySubText, runSupportAssetUpdatesForLauncherResult helpers
- Include forceOverride in yomitan anki settings cache key (was missing, causing incorrect cache hits)
- Detect same-PID stale stats daemon state to avoid self-connect
- Validate non-empty extension in buildFfmpegSubtitleExtractionArgs
- Drop unused message param from showOverlayLoadingStatusNotification
- Log and rethrow on session bindings artifact write failure
- Add unit tests for all extracted helpers

src/core/services/mpv.ts

@@ -45,6 +45,7 @@ export interface MpvRuntimeClientLike {
   playNextSubtitle?: () => void;
   setSubVisibility?: (visible: boolean) => void;
   setSecondarySubVisibility?: (visible: boolean) => void;
+  setCurrentSecondarySubText?: (text: string) => void;
 }
 
 export function showMpvOsdRuntime(

src/main.ts

@@ -2616,8 +2616,8 @@ function showYoutubeFlowStatusNotification(message: string): void {
   overlayNotificationsRuntime.showYoutubeFlowStatusNotification(message);
 }
 
-function showOverlayLoadingStatusNotification(message: string): void {
-  overlayNotificationsRuntime.showOverlayLoadingStatusNotification(message);
+function showOverlayLoadingStatusNotification(_message: string): void {
+  overlayNotificationsRuntime.showOverlayLoadingStatusNotification();
 }
 
 function dismissOverlayLoadingStatusNotification(): void {

src/main/password-store-args.ts

@@ -1,12 +1,15 @@
+const PASSWORD_STORE_ARG = '--password-store';
+const DEFAULT_LINUX_PASSWORD_STORE = 'gnome-libsecret';
+
 export function getPasswordStoreArg(argv: string[]): string | null {
   let resolved: string | null = null;
   for (let i = 0; i < argv.length; i += 1) {
     const arg = argv[i];
-    if (!arg?.startsWith('--password-store')) {
+    if (!arg?.startsWith(PASSWORD_STORE_ARG)) {
       continue;
     }
 
-    if (arg === '--password-store') {
+    if (arg === PASSWORD_STORE_ARG) {
       const value = argv[i + 1];
       if (value && !value.startsWith('--')) {
         resolved = value.trim();
@@ -16,7 +19,7 @@ export function getPasswordStoreArg(argv: string[]): string | null {
     }
 
     const [prefix, value] = arg.split('=', 2);
-    if (prefix === '--password-store' && value && value.trim().length > 0) {
+    if (prefix === PASSWORD_STORE_ARG && value && value.trim().length > 0) {
       resolved = value.trim();
     }
   }
@@ -26,11 +29,11 @@ export function getPasswordStoreArg(argv: string[]): string | null {
 export function normalizePasswordStoreArg(value: string): string {
   const normalized = value.trim();
   if (normalized.toLowerCase() === 'gnome') {
-    return 'gnome-libsecret';
+    return DEFAULT_LINUX_PASSWORD_STORE;
   }
   return normalized;
 }
 
 export function getDefaultPasswordStore(): string {
-  return 'gnome-libsecret';
+  return DEFAULT_LINUX_PASSWORD_STORE;
 }

src/main/runtime/autoplay-subtitle-priming-runtime.test.ts

@@ -0,0 +1,18 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import { setMpvCurrentSecondarySubText } from './autoplay-subtitle-priming-runtime';
+
+test('setMpvCurrentSecondarySubText uses client setter when available', () => {
+  const calls: string[] = [];
+  const client = {
+    currentSecondarySubText: '',
+    setCurrentSecondarySubText: (text: string) => {
+      calls.push(text);
+    },
+  };
+
+  setMpvCurrentSecondarySubText(client, 'secondary');
+
+  assert.deepEqual(calls, ['secondary']);
+  assert.equal(client.currentSecondarySubText, '');
+});

src/main/runtime/autoplay-subtitle-priming-runtime.ts

@@ -12,6 +12,7 @@ type AutoplaySubtitlePrimingMpvClient = {
   currentVideoPath?: string;
   currentTimePos?: number;
   currentSecondarySubText?: string;
+  setCurrentSecondarySubText?: (text: string) => void;
 };
 
 type AutoplaySubtitlePrimingPrefetchService = {
@@ -45,6 +46,20 @@ export interface AutoplaySubtitlePrimingRuntimeDeps {
   logDebug: (message: string) => void;
 }
 
+export function setMpvCurrentSecondarySubText(
+  client: Pick<
+    AutoplaySubtitlePrimingMpvClient,
+    'currentSecondarySubText' | 'setCurrentSecondarySubText'
+  >,
+  text: string,
+): void {
+  if (typeof client.setCurrentSecondarySubText === 'function') {
+    client.setCurrentSecondarySubText(text);
+    return;
+  }
+  client.currentSecondarySubText = text;
+}
+
 export function createAutoplaySubtitlePrimingRuntime(deps: AutoplaySubtitlePrimingRuntimeDeps) {
   const { subtitleProcessingController, emitSubtitlePayload } = deps;
 
@@ -137,7 +152,7 @@ export function createAutoplaySubtitlePrimingRuntime(deps: AutoplaySubtitlePrimi
       setCurrentSecondarySubText: (text) => {
         const client = deps.getMpvClient();
         if (client) {
-          client.currentSecondarySubText = text;
+          setMpvCurrentSecondarySubText(client, text);
         }
       },
       emitSecondarySubtitle: (text) => {

src/main/runtime/first-run-setup-plugin.test.ts

@@ -7,6 +7,7 @@ import {
   detectInstalledFirstRunPlugin,
   detectInstalledFirstRunPluginCandidates,
   detectInstalledMpvPlugin,
+  filterLegacyMpvPluginFileCandidates,
   removeLegacyMpvPluginCandidates,
   resolvePackagedFirstRunPluginAssets,
   resolvePackagedRuntimePluginPath,
@@ -220,6 +221,20 @@ test('detectInstalledMpvPlugin detects Linux legacy single-file plugin without v
   });
 });
 
+test('filterLegacyMpvPluginFileCandidates keeps only legacy file candidates', () => {
+  assert.deepEqual(
+    filterLegacyMpvPluginFileCandidates([
+      { path: '/tmp/mpv/scripts/subminer', kind: 'directory' },
+      { path: '/tmp/mpv/scripts/subminer.lua', kind: 'file' },
+      { path: '/tmp/mpv/scripts/subminer-loader.lua', kind: 'file' },
+    ]),
+    [
+      { path: '/tmp/mpv/scripts/subminer.lua', kind: 'file' },
+      { path: '/tmp/mpv/scripts/subminer-loader.lua', kind: 'file' },
+    ],
+  );
+});
+
 test('removeLegacyMpvPluginCandidates trashes candidates and reports partial failures', async () => {
   const calls: string[] = [];
   const result = await removeLegacyMpvPluginCandidates({

src/main/runtime/first-run-setup-plugin.ts

@@ -180,6 +180,12 @@ export function detectInstalledFirstRunPluginCandidates(options: {
   return candidates;
 }
 
+export function filterLegacyMpvPluginFileCandidates(
+  candidates: InstalledFirstRunPluginCandidate[],
+): InstalledFirstRunPluginCandidate[] {
+  return candidates.filter((candidate) => candidate.kind === 'file');
+}
+
 function parseInstalledPluginVersion(content: string): string | null {
   const match = content.match(/\bversion\s*=\s*["']([^"']+)["']/);
   return match?.[1] ?? null;

src/main/runtime/internal-subtitle-extraction.test.ts

@@ -0,0 +1,10 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import { buildFfmpegSubtitleExtractionArgs } from './internal-subtitle-extraction';
+
+test('buildFfmpegSubtitleExtractionArgs rejects output paths without an extension', () => {
+  assert.throws(
+    () => buildFfmpegSubtitleExtractionArgs('/tmp/video.mkv', 2, '/tmp/subtitle-output'),
+    /outputPath.*file extension/,
+  );
+});

src/main/runtime/internal-subtitle-extraction.ts

@@ -51,6 +51,10 @@ export function buildFfmpegSubtitleExtractionArgs(
   ffIndex: number,
   outputPath: string,
 ): string[] {
+  const outputFormat = path.extname(outputPath).slice(1);
+  if (!outputFormat) {
+    throw new Error(`outputPath must include a file extension for ffmpeg format: ${outputPath}`);
+  }
   return [
     '-hide_banner',
     '-nostdin',
@@ -64,7 +68,7 @@ export function buildFfmpegSubtitleExtractionArgs(
     '-map',
     `0:${ffIndex}`,
     '-f',
-    path.extname(outputPath).slice(1),
+    outputFormat,
     outputPath,
   ];
 }

src/main/runtime/overlay-notifications-runtime.ts

@@ -56,7 +56,7 @@ export function createOverlayNotificationsRuntime(deps: OverlayNotificationsRunt
   ) => void;
   showSubsyncStatusNotification: (message: string) => void;
   showYoutubeFlowStatusNotification: (message: string) => void;
-  showOverlayLoadingStatusNotification: (message: string) => void;
+  showOverlayLoadingStatusNotification: () => void;
   dismissOverlayLoadingStatusNotification: () => void;
   maybeStartOverlayLoadingOsd: (mediaPath?: string | null) => void;
 } {
@@ -213,8 +213,7 @@ export function createOverlayNotificationsRuntime(deps: OverlayNotificationsRunt
     return overlayLoadingOsdController;
   }
 
-  function showOverlayLoadingStatusNotification(message: string): void {
-    void message;
+  function showOverlayLoadingStatusNotification(): void {
     getOverlayLoadingOsdController().start();
   }
 
@@ -231,7 +230,7 @@ export function createOverlayNotificationsRuntime(deps: OverlayNotificationsRunt
     getVisibleOverlayRequested: () => deps.getVisibleOverlayVisible(),
     isOverlayContentReady: () => isVisibleOverlayContentReady(),
     startOverlayLoadingOsd: () => {
-      showOverlayLoadingStatusNotification('Overlay loading...');
+      showOverlayLoadingStatusNotification();
     },
   });
 

src/main/runtime/session-bindings-runtime.test.ts

@@ -0,0 +1,37 @@
+import assert from 'node:assert/strict';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import test from 'node:test';
+import type { CompiledSessionBinding, ResolvedConfig } from '../../types';
+import { createSessionBindingsRuntime } from './session-bindings-runtime';
+
+test('persistSessionBindings logs and does not publish bindings when artifact write fails', () => {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-session-bindings-runtime-'));
+  const configDir = path.join(root, 'config-file');
+  fs.writeFileSync(configDir, 'not a directory');
+  const calls: string[] = [];
+  const runtime = createSessionBindingsRuntime({
+    configDir,
+    getKeybindings: () => [],
+    getConfiguredShortcuts: () => ({ multiCopyTimeoutMs: 1500 }) as never,
+    getResolvedConfig: () =>
+      ({
+        stats: { toggleKey: 's', markWatchedKey: 'w' },
+      }) as ResolvedConfig,
+    getMpvClient: () => null,
+    setSessionBindings: () => calls.push('setSessionBindings'),
+    setSessionBindingsInitialized: () => calls.push('setSessionBindingsInitialized'),
+    logWarn: (message) => calls.push(`warn:${message}`),
+  });
+
+  try {
+    assert.throws(
+      () => runtime.persistSessionBindings([] as CompiledSessionBinding[]),
+      /ENOTDIR|EEXIST/,
+    );
+    assert.deepEqual(calls, ['warn:[session-bindings] Failed to write session bindings artifact']);
+  } finally {
+    fs.rmSync(root, { recursive: true, force: true });
+  }
+});

src/main/runtime/session-bindings-runtime.ts

@@ -54,7 +54,12 @@ export function createSessionBindingsRuntime(deps: SessionBindingsRuntimeDeps):
       warnings,
       numericSelectionTimeoutMs: deps.getConfiguredShortcuts().multiCopyTimeoutMs,
     });
+    try {
       writeSessionBindingsArtifact(deps.configDir, artifact);
+    } catch (error) {
+      deps.logWarn('[session-bindings] Failed to write session bindings artifact');
+      throw error;
+    }
     deps.setSessionBindings(bindings);
     deps.setSessionBindingsInitialized(true);
     const mpvClient = deps.getMpvClient();

src/main/runtime/stats-server-runtime.test.ts

@@ -0,0 +1,17 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import {
+  isSelfOwnedBackgroundStatsDaemonState,
+  shouldClearAppStateStatsServerOnStop,
+} from './stats-server-runtime';
+
+test('background stats daemon state owned by the current process is stale for stop flow', () => {
+  assert.equal(
+    isSelfOwnedBackgroundStatsDaemonState({ pid: process.pid, port: 6969, startedAtMs: 1 }),
+    true,
+  );
+});
+
+test('stats server app-state reference should be cleared after private server stop', () => {
+  assert.equal(shouldClearAppStateStatsServerOnStop({ hadStatsServer: true }), true);
+});

src/main/runtime/stats-server-runtime.ts

@@ -18,6 +18,20 @@ import {
 import { createEnsureStatsServerUrlHandler } from './stats-server-routing';
 import { shouldForceOverrideYomitanAnkiServer } from './yomitan-anki-server';
 
+export function isSelfOwnedBackgroundStatsDaemonState(state: {
+  pid: number;
+  port?: number;
+  startedAtMs?: number;
+}): boolean {
+  return state.pid === process.pid;
+}
+
+export function shouldClearAppStateStatsServerOnStop(options: {
+  hadStatsServer: boolean;
+}): boolean {
+  return options.hadStatsServer;
+}
+
 export interface StatsServerRuntimeDeps {
   userDataPath: string;
   statsDistPath: string;
@@ -90,6 +104,9 @@ export function createStatsServerRuntime(deps: StatsServerRuntimeDeps): {
     }
     statsServer.close();
     statsServer = null;
+    if (shouldClearAppStateStatsServerOnStop({ hadStatsServer: true })) {
+      deps.setAppStateStatsServer(null);
+    }
     clearOwnedBackgroundStatsDaemonState();
   }
 
@@ -198,6 +215,10 @@ export function createStatsServerRuntime(deps: StatsServerRuntimeDeps): {
       removeBackgroundStatsServerState(statsDaemonStatePath);
       return { ok: true, stale: true };
     }
+    if (isSelfOwnedBackgroundStatsDaemonState(state)) {
+      removeBackgroundStatsServerState(statsDaemonStatePath);
+      return { ok: true, stale: true };
+    }
     if (!isBackgroundStatsServerProcessAlive(state.pid)) {
       removeBackgroundStatsServerState(statsDaemonStatePath);
       return { ok: true, stale: true };

src/main/runtime/update/update-service-runtime.test.ts

@@ -0,0 +1,20 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import { runSupportAssetUpdatesForLauncherResult } from './update-support-assets-runtime';
+
+test('runSupportAssetUpdatesForLauncherResult logs support-asset errors and preserves launcher result', async () => {
+  const warnings: string[] = [];
+  const launcherResult = { status: 'updated' } as const;
+  const result = await runSupportAssetUpdatesForLauncherResult({
+    launcherResult,
+    updateSupportAssets: async () => {
+      throw new Error('archive failed');
+    },
+    logWarn: (message, details) => {
+      warnings.push(`${message}:${details instanceof Error ? details.message : String(details)}`);
+    },
+  });
+
+  assert.equal(result, launcherResult);
+  assert.deepEqual(warnings, ['Support asset update failed after launcher update:archive failed']);
+});

src/main/runtime/update/update-service-runtime.ts

@@ -21,6 +21,7 @@ import { notifyUpdateAvailable } from './update-notifications';
 import { createUpdateDialogPresenter } from './update-dialogs';
 import { createFileUpdateStateStore, createUpdateService } from './update-service';
 import { updateSupportAssetsFromRelease } from './support-assets';
+import { runSupportAssetUpdatesForLauncherResult } from './update-support-assets-runtime';
 
 const SUBMINER_BUNDLE_ID = 'com.sudacode.SubMiner';
 
@@ -79,19 +80,16 @@ export function createUpdateServiceRuntime(deps: UpdateServiceRuntimeDeps): {
       launcherPath,
       downloadAsset: (url) => fetchReleaseAssetBuffer(fetchForUpdater, url),
     });
-    const supportResults = await updateSupportAssetsFromRelease({
+    return runSupportAssetUpdatesForLauncherResult({
+      launcherResult,
+      updateSupportAssets: () =>
+        updateSupportAssetsFromRelease({
           release,
           sha256Sums: sums,
           downloadAsset: (url) => fetchReleaseAssetBuffer(fetchForUpdater, url),
+        }),
+      logWarn: (message, details) => deps.logWarn(message, details),
     });
-    for (const result of supportResults) {
-      if (result.status === 'protected' && result.command) {
-        deps.logWarn(`Rofi theme update requires manual command: ${result.command}`);
-      } else if (result.status === 'hash-mismatch' || result.status === 'missing-asset') {
-        deps.logWarn(`Rofi theme update skipped: ${result.message ?? result.status}`);
-      }
-    }
-    return launcherResult;
   }
 
   function getUpdateService() {

src/main/runtime/update/update-support-assets-runtime.ts

@@ -0,0 +1,22 @@
+export async function runSupportAssetUpdatesForLauncherResult<
+  TLauncherResult,
+  TSupportResult extends { status: string; command?: string; message?: string },
+>(options: {
+  launcherResult: TLauncherResult;
+  updateSupportAssets: () => Promise<TSupportResult[]>;
+  logWarn: (message: string, details?: unknown) => void;
+}): Promise<TLauncherResult> {
+  try {
+    const supportResults = await options.updateSupportAssets();
+    for (const result of supportResults) {
+      if (result.status === 'protected' && result.command) {
+        options.logWarn(`Rofi theme update requires manual command: ${result.command}`);
+      } else if (result.status === 'hash-mismatch' || result.status === 'missing-asset') {
+        options.logWarn(`Rofi theme update skipped: ${result.message ?? result.status}`);
+      }
+    }
+  } catch (error) {
+    options.logWarn('Support asset update failed after launcher update', error);
+  }
+  return options.launcherResult;
+}

src/main/runtime/windows-mpv-plugin-detection.ts

@@ -3,6 +3,7 @@ import * as os from 'os';
 import {
   detectInstalledFirstRunPluginCandidates,
   detectInstalledMpvPlugin,
+  filterLegacyMpvPluginFileCandidates,
   removeLegacyMpvPluginCandidates,
   resolvePackagedRuntimePluginPath,
 } from './first-run-setup-plugin';
@@ -86,12 +87,14 @@ export function createWindowsMpvPluginDetectionRuntime(
     }
 
     const result = await removeLegacyMpvPluginCandidates({
-      candidates: detectInstalledFirstRunPluginCandidates({
+      candidates: filterLegacyMpvPluginFileCandidates(
+        detectInstalledFirstRunPluginCandidates({
           platform: 'win32',
           homeDir: os.homedir(),
           appDataDir: app.getPath('appData'),
           mpvExecutablePath: mpvPath,
         }),
+      ),
       trashItem: (candidatePath) => shell.trashItem(candidatePath),
     });
     if (result.ok) {

src/main/runtime/yomitan-anki-server-sync.test.ts

@@ -0,0 +1,18 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import { buildYomitanAnkiSettingsKey } from './yomitan-anki-server-sync';
+
+test('buildYomitanAnkiSettingsKey includes force override policy', () => {
+  assert.notEqual(
+    buildYomitanAnkiSettingsKey({
+      targetUrl: 'http://127.0.0.1:8766',
+      targetDeck: 'Mining',
+      forceOverride: false,
+    }),
+    buildYomitanAnkiSettingsKey({
+      targetUrl: 'http://127.0.0.1:8766',
+      targetDeck: 'Mining',
+      forceOverride: true,
+    }),
+  );
+});

src/main/runtime/yomitan-anki-server-sync.ts

@@ -13,6 +13,14 @@ export interface YomitanAnkiServerSyncRuntimeDeps {
   logInfo: (message: string, ...args: unknown[]) => void;
 }
 
+export function buildYomitanAnkiSettingsKey(options: {
+  targetUrl: string;
+  targetDeck: string;
+  forceOverride: boolean;
+}): string {
+  return `${options.targetUrl}\n${options.targetDeck}\nforceOverride:${options.forceOverride}`;
+}
+
 export function createYomitanAnkiServerSyncRuntime(deps: YomitanAnkiServerSyncRuntimeDeps): {
   syncYomitanDefaultProfileAnkiServer: () => Promise<void>;
 } {
@@ -30,7 +38,14 @@ export function createYomitanAnkiServerSyncRuntime(deps: YomitanAnkiServerSyncRu
     const targetUrl = getPreferredYomitanAnkiServerUrl().trim();
     const ankiConnectConfig = deps.getResolvedConfig().ankiConnect;
     const targetDeck = ankiConnectConfig?.deck?.trim() ?? '';
-    const targetSettingsKey = `${targetUrl}\n${targetDeck}`;
+    const forceOverride = ankiConnectConfig
+      ? shouldForceOverrideYomitanAnkiServer(ankiConnectConfig)
+      : false;
+    const targetSettingsKey = buildYomitanAnkiSettingsKey({
+      targetUrl,
+      targetDeck,
+      forceOverride,
+    });
     if (!targetUrl || targetSettingsKey === lastSyncedYomitanAnkiSettingsKey) {
       return;
     }
@@ -47,9 +62,7 @@ export function createYomitanAnkiServerSyncRuntime(deps: YomitanAnkiServerSyncRu
         },
       },
       {
-        forceOverride: ankiConnectConfig
-          ? shouldForceOverrideYomitanAnkiServer(ankiConnectConfig)
-          : false,
+        forceOverride,
         deck: targetDeck,
       },
     );