fix(anki): address latest PR 19 review follow-ups

launcher/mpv.test.ts

@@ -245,6 +245,44 @@ function makeExecutable(filePath: string): void {
   fs.chmodSync(filePath, 0o755);
 }
 
+function withFindAppBinaryEnvSandbox(run: () => void): void {
+  const originalAppImagePath = process.env.SUBMINER_APPIMAGE_PATH;
+  const originalBinaryPath = process.env.SUBMINER_BINARY_PATH;
+  try {
+    delete process.env.SUBMINER_APPIMAGE_PATH;
+    delete process.env.SUBMINER_BINARY_PATH;
+    run();
+  } finally {
+    if (originalAppImagePath === undefined) {
+      delete process.env.SUBMINER_APPIMAGE_PATH;
+    } else {
+      process.env.SUBMINER_APPIMAGE_PATH = originalAppImagePath;
+    }
+    if (originalBinaryPath === undefined) {
+      delete process.env.SUBMINER_BINARY_PATH;
+    } else {
+      process.env.SUBMINER_BINARY_PATH = originalBinaryPath;
+    }
+  }
+}
+
+function withAccessSyncStub(isExecutablePath: (filePath: string) => boolean, run: () => void): void {
+  const originalAccessSync = fs.accessSync;
+  try {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    (fs as any).accessSync = (filePath: string): void => {
+      if (isExecutablePath(filePath)) {
+        return;
+      }
+      throw Object.assign(new Error(`EACCES: ${filePath}`), { code: 'EACCES' });
+    };
+    run();
+  } finally {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    (fs as any).accessSync = originalAccessSync;
+  }
+}
+
 test('findAppBinary resolves ~/.local/bin/SubMiner.AppImage when it exists', () => {
   const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-test-home-'));
   const originalHomedir = os.homedir;
@@ -253,8 +291,10 @@ test('findAppBinary resolves ~/.local/bin/SubMiner.AppImage when it exists', ()
     const appImage = path.join(baseDir, '.local/bin/SubMiner.AppImage');
     makeExecutable(appImage);
 
-    const result = findAppBinary('/some/other/path/subminer');
-    assert.equal(result, appImage);
+    withFindAppBinaryEnvSandbox(() => {
+      const result = findAppBinary('/some/other/path/subminer');
+      assert.equal(result, appImage);
+    });
   } finally {
     os.homedir = originalHomedir;
     fs.rmSync(baseDir, { recursive: true, force: true });
@@ -264,22 +304,16 @@ test('findAppBinary resolves ~/.local/bin/SubMiner.AppImage when it exists', ()
 test('findAppBinary resolves /opt/SubMiner/SubMiner.AppImage when ~/.local/bin candidate does not exist', () => {
   const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-test-home-'));
   const originalHomedir = os.homedir;
-  const originalAccessSync = fs.accessSync;
   try {
     os.homedir = () => baseDir;
-    // No ~/.local/bin/SubMiner.AppImage; patch accessSync so only /opt path is executable
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    (fs as any).accessSync = (filePath: string, mode?: number): void => {
-      if (filePath === '/opt/SubMiner/SubMiner.AppImage') return;
-      throw Object.assign(new Error(`EACCES: ${filePath}`), { code: 'EACCES' });
-    };
-
-    const result = findAppBinary('/some/other/path/subminer');
-    assert.equal(result, '/opt/SubMiner/SubMiner.AppImage');
+    withFindAppBinaryEnvSandbox(() => {
+      withAccessSyncStub((filePath) => filePath === '/opt/SubMiner/SubMiner.AppImage', () => {
+        const result = findAppBinary('/some/other/path/subminer');
+        assert.equal(result, '/opt/SubMiner/SubMiner.AppImage');
+      });
+    });
   } finally {
     os.homedir = originalHomedir;
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    (fs as any).accessSync = originalAccessSync;
     fs.rmSync(baseDir, { recursive: true, force: true });
   }
 });
@@ -296,9 +330,13 @@ test('findAppBinary finds subminer on PATH when AppImage candidates do not exist
     makeExecutable(wrapperPath);
     process.env.PATH = `${binDir}${path.delimiter}${originalPath ?? ''}`;
 
-    // selfPath must differ from wrapperPath so the self-check does not exclude it
-    const result = findAppBinary(path.join(baseDir, 'launcher', 'subminer'));
-    assert.equal(result, wrapperPath);
+    withFindAppBinaryEnvSandbox(() => {
+      withAccessSyncStub((filePath) => filePath === wrapperPath, () => {
+        // selfPath must differ from wrapperPath so the self-check does not exclude it
+        const result = findAppBinary(path.join(baseDir, 'launcher', 'subminer'));
+        assert.equal(result, wrapperPath);
+      });
+    });
   } finally {
     os.homedir = originalHomedir;
     process.env.PATH = originalPath;

src/anki-integration.test.ts

@@ -250,6 +250,34 @@ test('AnkiIntegration does not allocate proxy server when proxy transport is dis
   assert.equal(privateState.runtime.proxyServer, null);
 });
 
+test('AnkiIntegration marks partial update notifications as failures in OSD mode', async () => {
+  const osdMessages: string[] = [];
+  const integration = new AnkiIntegration(
+    {
+      behavior: {
+        notificationType: 'osd',
+      },
+    },
+    {} as never,
+    {} as never,
+    (text) => {
+      osdMessages.push(text);
+    },
+  );
+
+  await (
+    integration as unknown as {
+      showNotification: (
+        noteId: number,
+        label: string | number,
+        errorSuffix?: string,
+      ) => Promise<void>;
+    }
+  ).showNotification(42, 'taberu', 'image failed');
+
+  assert.deepEqual(osdMessages, ['x Updated card: taberu (image failed)']);
+});
+
 test('FieldGroupingMergeCollaborator synchronizes ExpressionAudio from merged SentenceAudio', async () => {
   const collaborator = createFieldGroupingMergeCollaborator();
 

src/anki-integration.ts

@@ -913,7 +913,7 @@ export class AnkiIntegration {
     const type = this.config.behavior?.notificationType || 'osd';
 
     if (type === 'osd' || type === 'both') {
-      this.showUpdateResult(message, true);
+      this.showUpdateResult(message, errorSuffix === undefined);
     } else {
       this.clearUpdateProgress();
     }

src/anki-integration/known-word-cache.test.ts

@@ -0,0 +1,112 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+
+import type { AnkiConnectConfig } from '../types';
+import { KnownWordCacheManager } from './known-word-cache';
+
+function createKnownWordCacheHarness(config: AnkiConnectConfig): {
+  manager: KnownWordCacheManager;
+  cleanup: () => void;
+} {
+  const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-known-word-cache-'));
+  const statePath = path.join(stateDir, 'known-words-cache.json');
+  const manager = new KnownWordCacheManager({
+    client: {
+      findNotes: async () => [],
+      notesInfo: async () => [],
+    },
+    getConfig: () => config,
+    knownWordCacheStatePath: statePath,
+    showStatusNotification: () => undefined,
+  });
+
+  return {
+    manager,
+    cleanup: () => {
+      fs.rmSync(stateDir, { recursive: true, force: true });
+    },
+  };
+}
+
+test('KnownWordCacheManager invalidates persisted cache when fields.word changes', () => {
+  const config: AnkiConnectConfig = {
+    deck: 'Mining',
+    fields: {
+      word: 'Word',
+    },
+    knownWords: {
+      highlightEnabled: true,
+    },
+  };
+  const { manager, cleanup } = createKnownWordCacheHarness(config);
+
+  try {
+    manager.appendFromNoteInfo({
+      noteId: 1,
+      fields: {
+        Word: { value: '猫' },
+      },
+    });
+    assert.equal(manager.isKnownWord('猫'), true);
+
+    config.fields = {
+      ...config.fields,
+      word: 'Expression',
+    };
+
+    (
+      manager as unknown as {
+        loadKnownWordCacheState: () => void;
+      }
+    ).loadKnownWordCacheState();
+
+    assert.equal(manager.isKnownWord('猫'), false);
+  } finally {
+    cleanup();
+  }
+});
+
+test('KnownWordCacheManager invalidates persisted cache when per-deck fields change', () => {
+  const config: AnkiConnectConfig = {
+    fields: {
+      word: 'Word',
+    },
+    knownWords: {
+      highlightEnabled: true,
+      decks: {
+        Mining: ['Word'],
+      },
+    },
+  };
+  const { manager, cleanup } = createKnownWordCacheHarness(config);
+
+  try {
+    manager.appendFromNoteInfo({
+      noteId: 1,
+      fields: {
+        Word: { value: '猫' },
+      },
+    });
+    assert.equal(manager.isKnownWord('猫'), true);
+
+    config.knownWords = {
+      ...config.knownWords,
+      decks: {
+        Mining: ['Expression'],
+      },
+    };
+
+    (
+      manager as unknown as {
+        loadKnownWordCacheState: () => void;
+      }
+    ).loadKnownWordCacheState();
+
+    assert.equal(manager.isKnownWord('猫'), false);
+  } finally {
+    cleanup();
+  }
+});

src/anki-integration/known-word-cache.ts

@@ -8,6 +8,57 @@ import { createLogger } from '../logger';
 
 const log = createLogger('anki').child('integration.known-word-cache');
 
+function trimToNonEmptyString(value: unknown): string | null {
+  if (typeof value !== 'string') return null;
+  const trimmed = value.trim();
+  return trimmed.length > 0 ? trimmed : null;
+}
+
+export function getKnownWordCacheRefreshIntervalMinutes(config: AnkiConnectConfig): number {
+  const refreshMinutes = config.knownWords?.refreshMinutes;
+  return typeof refreshMinutes === 'number' && Number.isFinite(refreshMinutes) && refreshMinutes > 0
+    ? refreshMinutes
+    : DEFAULT_ANKI_CONNECT_CONFIG.knownWords.refreshMinutes;
+}
+
+export function getKnownWordCacheScopeForConfig(config: AnkiConnectConfig): string {
+  const configuredDecks = config.knownWords?.decks;
+  if (configuredDecks && typeof configuredDecks === 'object' && !Array.isArray(configuredDecks)) {
+    const normalizedDecks = Object.entries(configuredDecks)
+      .map(([deckName, fields]) => {
+        const name = trimToNonEmptyString(deckName);
+        if (!name) return null;
+        const normalizedFields = Array.isArray(fields)
+          ? [
+              ...new Set(
+                fields
+                  .map(String)
+                  .map(trimToNonEmptyString)
+                  .filter((field): field is string => Boolean(field)),
+              ),
+            ].sort()
+          : [];
+        return [name, normalizedFields];
+      })
+      .filter((entry): entry is [string, string[]] => entry !== null)
+      .sort(([a], [b]) => a.localeCompare(b));
+    if (normalizedDecks.length > 0) {
+      return `decks:${JSON.stringify(normalizedDecks)}`;
+    }
+  }
+
+  const configuredDeck = trimToNonEmptyString(config.deck);
+  return configuredDeck ? `deck:${configuredDeck}` : 'is:note';
+}
+
+export function getKnownWordCacheLifecycleConfig(config: AnkiConnectConfig): string {
+  return JSON.stringify({
+    refreshMinutes: getKnownWordCacheRefreshIntervalMinutes(config),
+    scope: getKnownWordCacheScopeForConfig(config),
+    fieldsWord: trimToNonEmptyString(config.fields?.word) ?? '',
+  });
+}
+
 export interface KnownWordCacheNoteInfo {
   noteId: number;
   fields: Record<string, { value: string }>;
@@ -39,7 +90,7 @@ interface KnownWordCacheDeps {
 
 export class KnownWordCacheManager {
   private knownWordsLastRefreshedAtMs = 0;
-  private knownWordsScope = '';
+  private knownWordsStateKey = '';
   private knownWords: Set<string> = new Set();
   private knownWordsRefreshTimer: ReturnType<typeof setInterval> | null = null;
   private isRefreshingKnownWords = false;
@@ -73,7 +124,7 @@ export class KnownWordCacheManager {
     }
 
     const refreshMinutes = this.getKnownWordRefreshIntervalMs() / 60_000;
-    const scope = this.getKnownWordCacheScope();
+    const scope = getKnownWordCacheScopeForConfig(this.deps.getConfig());
     log.info(
       'Known-word cache lifecycle enabled',
       `scope=${scope}`,
@@ -101,12 +152,12 @@ export class KnownWordCacheManager {
       return;
     }
 
-    const currentScope = this.getKnownWordCacheScope();
-    if (this.knownWordsScope && this.knownWordsScope !== currentScope) {
+    const currentStateKey = this.getKnownWordCacheStateKey();
+    if (this.knownWordsStateKey && this.knownWordsStateKey !== currentStateKey) {
       this.clearKnownWordCacheState();
     }
-    if (!this.knownWordsScope) {
-      this.knownWordsScope = currentScope;
+    if (!this.knownWordsStateKey) {
+      this.knownWordsStateKey = currentStateKey;
     }
 
     let addedCount = 0;
@@ -127,7 +178,7 @@ export class KnownWordCacheManager {
       log.info(
         'Known-word cache updated in-session',
         `added=${addedCount}`,
-        `scope=${currentScope}`,
+        `scope=${getKnownWordCacheScopeForConfig(this.deps.getConfig())}`,
       );
     }
   }
@@ -135,7 +186,7 @@ export class KnownWordCacheManager {
   clearKnownWordCacheState(): void {
     this.knownWords = new Set();
     this.knownWordsLastRefreshedAtMs = 0;
-    this.knownWordsScope = this.getKnownWordCacheScope();
+    this.knownWordsStateKey = this.getKnownWordCacheStateKey();
     try {
       if (fs.existsSync(this.statePath)) {
         fs.unlinkSync(this.statePath);
@@ -188,7 +239,7 @@ export class KnownWordCacheManager {
 
       this.knownWords = nextKnownWords;
       this.knownWordsLastRefreshedAtMs = Date.now();
-      this.knownWordsScope = this.getKnownWordCacheScope();
+      this.knownWordsStateKey = this.getKnownWordCacheStateKey();
       this.persistKnownWordCacheState();
       log.info(
         'Known-word cache refreshed',
@@ -208,12 +259,7 @@ export class KnownWordCacheManager {
   }
 
   private getKnownWordRefreshIntervalMs(): number {
-    const minutes = this.deps.getConfig().knownWords?.refreshMinutes;
-    const safeMinutes =
-      typeof minutes === 'number' && Number.isFinite(minutes) && minutes > 0
-        ? minutes
-        : DEFAULT_ANKI_CONNECT_CONFIG.knownWords.refreshMinutes;
-    return safeMinutes * 60_000;
+    return getKnownWordCacheRefreshIntervalMinutes(this.deps.getConfig()) * 60_000;
   }
 
   private getKnownWordDecks(): string[] {
@@ -259,19 +305,15 @@ export class KnownWordCacheManager {
     return `(${deckQueries.join(' OR ')})`;
   }
 
-  private getKnownWordCacheScope(): string {
-    const decks = this.getKnownWordDecks();
-    if (decks.length === 0) {
-      return 'is:note';
-    }
-    return `decks:${JSON.stringify(decks)}`;
+  private getKnownWordCacheStateKey(): string {
+    return getKnownWordCacheLifecycleConfig(this.deps.getConfig());
   }
 
   private isKnownWordCacheStale(): boolean {
     if (!this.isKnownWordCacheEnabled()) {
       return true;
     }
-    if (this.knownWordsScope !== this.getKnownWordCacheScope()) {
+    if (this.knownWordsStateKey !== this.getKnownWordCacheStateKey()) {
       return true;
     }
     if (this.knownWordsLastRefreshedAtMs <= 0) {
@@ -285,7 +327,7 @@ export class KnownWordCacheManager {
       if (!fs.existsSync(this.statePath)) {
         this.knownWords = new Set();
         this.knownWordsLastRefreshedAtMs = 0;
-        this.knownWordsScope = this.getKnownWordCacheScope();
+        this.knownWordsStateKey = this.getKnownWordCacheStateKey();
         return;
       }
 
@@ -293,7 +335,7 @@ export class KnownWordCacheManager {
       if (!raw.trim()) {
         this.knownWords = new Set();
         this.knownWordsLastRefreshedAtMs = 0;
-        this.knownWordsScope = this.getKnownWordCacheScope();
+        this.knownWordsStateKey = this.getKnownWordCacheStateKey();
         return;
       }
 
@@ -301,14 +343,14 @@ export class KnownWordCacheManager {
       if (!this.isKnownWordCacheStateValid(parsed)) {
         this.knownWords = new Set();
         this.knownWordsLastRefreshedAtMs = 0;
-        this.knownWordsScope = this.getKnownWordCacheScope();
+        this.knownWordsStateKey = this.getKnownWordCacheStateKey();
         return;
       }
 
-      if (parsed.scope !== this.getKnownWordCacheScope()) {
+      if (parsed.scope !== this.getKnownWordCacheStateKey()) {
         this.knownWords = new Set();
         this.knownWordsLastRefreshedAtMs = 0;
-        this.knownWordsScope = this.getKnownWordCacheScope();
+        this.knownWordsStateKey = this.getKnownWordCacheStateKey();
         return;
       }
 
@@ -322,12 +364,12 @@ export class KnownWordCacheManager {
 
       this.knownWords = nextKnownWords;
       this.knownWordsLastRefreshedAtMs = parsed.refreshedAtMs;
-      this.knownWordsScope = parsed.scope;
+      this.knownWordsStateKey = parsed.scope;
     } catch (error) {
       log.warn('Failed to load known-word cache state:', (error as Error).message);
       this.knownWords = new Set();
       this.knownWordsLastRefreshedAtMs = 0;
-      this.knownWordsScope = this.getKnownWordCacheScope();
+      this.knownWordsStateKey = this.getKnownWordCacheStateKey();
     }
   }
 
@@ -336,7 +378,7 @@ export class KnownWordCacheManager {
       const state: KnownWordCacheState = {
         version: 1,
         refreshedAtMs: this.knownWordsLastRefreshedAtMs,
-        scope: this.knownWordsScope,
+        scope: this.knownWordsStateKey,
         words: Array.from(this.knownWords),
       };
       fs.writeFileSync(this.statePath, JSON.stringify(state), 'utf-8');

src/anki-integration/runtime.test.ts

@@ -151,3 +151,36 @@ test('AnkiIntegrationRuntime restarts known-word lifecycle when known-word setti
 
   assert.deepEqual(calls, ['known:start']);
 });
+
+test('AnkiIntegrationRuntime does not stop lifecycle when disabled while runtime is stopped', () => {
+  const { runtime, calls } = createRuntime({
+    knownWords: {
+      highlightEnabled: true,
+    },
+  });
+
+  runtime.applyRuntimeConfigPatch({
+    knownWords: {
+      highlightEnabled: false,
+    },
+  });
+
+  assert.deepEqual(calls, ['known:clear']);
+});
+
+test('AnkiIntegrationRuntime does not restart known-word lifecycle for config changes while stopped', () => {
+  const { runtime, calls } = createRuntime({
+    knownWords: {
+      highlightEnabled: true,
+      refreshMinutes: 90,
+    },
+  });
+
+  runtime.applyRuntimeConfigPatch({
+    knownWords: {
+      refreshMinutes: 120,
+    },
+  });
+
+  assert.deepEqual(calls, []);
+});

src/anki-integration/runtime.ts

@@ -1,5 +1,10 @@
 import { DEFAULT_ANKI_CONNECT_CONFIG } from '../config';
 import type { AnkiConnectConfig } from '../types';
+import {
+  getKnownWordCacheLifecycleConfig,
+  getKnownWordCacheRefreshIntervalMinutes,
+  getKnownWordCacheScopeForConfig,
+} from './known-word-cache';
 
 export interface AnkiIntegrationRuntimeProxyServer {
   start(options: { host: string; port: number; upstreamUrl: string }): void;
@@ -196,12 +201,15 @@ export class AnkiIntegrationRuntime {
     this.deps.onConfigChanged?.(this.config);
     const nextKnownWordCacheEnabled = this.config.knownWords?.highlightEnabled === true;
 
-    if (wasKnownWordCacheEnabled && this.config.knownWords?.highlightEnabled === false) {
-      this.deps.knownWordCache.stopLifecycle();
+    if (wasKnownWordCacheEnabled && !nextKnownWordCacheEnabled) {
+      if (this.started) {
+        this.deps.knownWordCache.stopLifecycle();
+      }
       this.deps.knownWordCache.clearKnownWordCacheState();
-    } else if (!wasKnownWordCacheEnabled && nextKnownWordCacheEnabled) {
+    } else if (this.started && !wasKnownWordCacheEnabled && nextKnownWordCacheEnabled) {
       this.deps.knownWordCache.startLifecycle();
     } else if (
+      this.started &&
       wasKnownWordCacheEnabled &&
       nextKnownWordCacheEnabled &&
       previousKnownWordCacheConfig !== null &&
@@ -218,45 +226,15 @@ export class AnkiIntegrationRuntime {
   }
 
   private getKnownWordCacheLifecycleConfig(config: AnkiConnectConfig): string {
-    return JSON.stringify({
-      refreshMinutes: this.getKnownWordRefreshIntervalMinutes(config),
-      scope: this.getKnownWordCacheScopeForConfig(config),
-      fieldsWord: trimToNonEmptyString(config.fields?.word) ?? '',
-    });
+    return getKnownWordCacheLifecycleConfig(config);
   }
 
   private getKnownWordRefreshIntervalMinutes(config: AnkiConnectConfig): number {
-    const refreshMinutes = config.knownWords?.refreshMinutes;
-    return typeof refreshMinutes === 'number' && Number.isFinite(refreshMinutes) && refreshMinutes > 0
-      ? refreshMinutes
-      : DEFAULT_ANKI_CONNECT_CONFIG.knownWords.refreshMinutes;
+    return getKnownWordCacheRefreshIntervalMinutes(config);
   }
 
   private getKnownWordCacheScopeForConfig(config: AnkiConnectConfig): string {
-    const configuredDecks = config.knownWords?.decks;
-    if (configuredDecks && typeof configuredDecks === 'object' && !Array.isArray(configuredDecks)) {
-      const normalizedDecks = Object.entries(configuredDecks)
-        .map(([deckName, fields]) => {
-          const name = trimToNonEmptyString(deckName);
-          if (!name) return null;
-          const normalizedFields = Array.isArray(fields)
-            ? [
-                ...new Set(
-                  fields.map(String).map(trimToNonEmptyString).filter((field): field is string => Boolean(field)),
-                ),
-              ].sort()
-            : [];
-          return [name, normalizedFields];
-        })
-        .filter((entry): entry is [string, string[]] => entry !== null)
-        .sort(([a], [b]) => a.localeCompare(b));
-      if (normalizedDecks.length > 0) {
-        return `decks:${JSON.stringify(normalizedDecks)}`;
-      }
-    }
-
-    const configuredDeck = trimToNonEmptyString(config.deck);
-    return configuredDeck ? `deck:${configuredDeck}` : 'is:note';
+    return getKnownWordCacheScopeForConfig(config);
   }
 
   getOrCreateProxyServer(): AnkiIntegrationRuntimeProxyServer {