Address second CodeRabbit review round

plugin/subminer/session_bindings.lua

@@ -262,20 +262,24 @@ function M.create(ctx)
 	end
 
 	local function register_bindings()
-		clear_bindings()
-
 		local artifact, load_error = load_artifact()
 		if not artifact then
 			subminer_log("warn", "session-bindings", load_error)
 			return false
 		end
 
+		clear_numeric_selection(false)
+
+		local previous_binding_names = state.session_binding_names
+		local next_binding_names = {}
+		state.session_binding_names = next_binding_names
+
 		local timeout_ms = tonumber(artifact.numericSelectionTimeoutMs) or 3000
 		for index, binding in ipairs(artifact.bindings) do
 			local key_name = key_spec_to_mpv_binding(binding.key)
 			if key_name then
 				local name = "subminer-session-binding-" .. tostring(index)
-				state.session_binding_names[#state.session_binding_names + 1] = name
+				next_binding_names[#next_binding_names + 1] = name
 				mp.add_forced_key_binding(key_name, name, function()
 					handle_binding(binding, timeout_ms)
 				end)
@@ -288,10 +292,12 @@ function M.create(ctx)
 			end
 		end
 
+		remove_binding_names(previous_binding_names)
+
 		subminer_log(
 			"info",
 			"session-bindings",
-			"Registered " .. tostring(#state.session_binding_names) .. " shared session bindings"
+			"Registered " .. tostring(#next_binding_names) .. " shared session bindings"
 		)
 		return true
 	end

src/core/services/overlay-shortcut-handler.test.ts

@@ -6,7 +6,11 @@ import {
   OverlayShortcutRuntimeDeps,
   runOverlayShortcutLocalFallback,
 } from './overlay-shortcut-handler';
-import { shouldActivateOverlayShortcuts } from './overlay-shortcut';
+import {
+  registerOverlayShortcutsRuntime,
+  shouldActivateOverlayShortcuts,
+  unregisterOverlayShortcutsRuntime,
+} from './overlay-shortcut';
 
 function makeShortcuts(overrides: Partial<ConfiguredShortcuts> = {}): ConfiguredShortcuts {
   return {
@@ -313,3 +317,85 @@ test('shouldActivateOverlayShortcuts preserves non-macOS behavior', () => {
     true,
   );
 });
+
+test('registerOverlayShortcutsRuntime reports active shortcuts when configured', () => {
+  const result = registerOverlayShortcutsRuntime({
+    getConfiguredShortcuts: () =>
+      ({
+        toggleVisibleOverlayGlobal: null,
+        copySubtitle: null,
+        copySubtitleMultiple: null,
+        updateLastCardFromClipboard: null,
+        triggerFieldGrouping: null,
+        triggerSubsync: null,
+        mineSentence: null,
+        mineSentenceMultiple: null,
+        multiCopyTimeoutMs: 2500,
+        toggleSecondarySub: null,
+        markAudioCard: null,
+        openRuntimeOptions: null,
+        openJimaku: 'Ctrl+J',
+      }) as never,
+    getOverlayHandlers: () => ({
+      copySubtitle: () => {},
+      copySubtitleMultiple: () => {},
+      updateLastCardFromClipboard: () => {},
+      triggerFieldGrouping: () => {},
+      triggerSubsync: () => {},
+      mineSentence: () => {},
+      mineSentenceMultiple: () => {},
+      toggleSecondarySub: () => {},
+      markAudioCard: () => {},
+      openRuntimeOptions: () => {},
+      openJimaku: () => {},
+    }),
+    cancelPendingMultiCopy: () => {},
+    cancelPendingMineSentenceMultiple: () => {},
+  });
+
+  assert.equal(result, true);
+});
+
+test('unregisterOverlayShortcutsRuntime clears pending shortcut work when active', () => {
+  const calls: string[] = [];
+  const result = unregisterOverlayShortcutsRuntime(true, {
+    getConfiguredShortcuts: () =>
+      ({
+        toggleVisibleOverlayGlobal: null,
+        copySubtitle: null,
+        copySubtitleMultiple: null,
+        updateLastCardFromClipboard: null,
+        triggerFieldGrouping: null,
+        triggerSubsync: null,
+        mineSentence: null,
+        mineSentenceMultiple: null,
+        multiCopyTimeoutMs: 2500,
+        toggleSecondarySub: null,
+        markAudioCard: null,
+        openRuntimeOptions: null,
+        openJimaku: null,
+      }) as never,
+    getOverlayHandlers: () => ({
+      copySubtitle: () => {},
+      copySubtitleMultiple: () => {},
+      updateLastCardFromClipboard: () => {},
+      triggerFieldGrouping: () => {},
+      triggerSubsync: () => {},
+      mineSentence: () => {},
+      mineSentenceMultiple: () => {},
+      toggleSecondarySub: () => {},
+      markAudioCard: () => {},
+      openRuntimeOptions: () => {},
+      openJimaku: () => {},
+    }),
+    cancelPendingMultiCopy: () => {
+      calls.push('cancel-multi-copy');
+    },
+    cancelPendingMineSentenceMultiple: () => {
+      calls.push('cancel-mine-sentence-multiple');
+    },
+  });
+
+  assert.equal(result, false);
+  assert.deepEqual(calls, ['cancel-multi-copy', 'cancel-mine-sentence-multiple']);
+});

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

@@ -0,0 +1,94 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import type { ConfiguredShortcuts } from '../utils/shortcut-config';
+import {
+  registerOverlayShortcuts,
+  syncOverlayShortcutsRuntime,
+  unregisterOverlayShortcutsRuntime,
+} from './overlay-shortcut';
+
+function createShortcuts(overrides: Partial<ConfiguredShortcuts> = {}): ConfiguredShortcuts {
+  return {
+    toggleVisibleOverlayGlobal: null,
+    copySubtitle: null,
+    copySubtitleMultiple: null,
+    updateLastCardFromClipboard: null,
+    triggerFieldGrouping: null,
+    triggerSubsync: null,
+    mineSentence: null,
+    mineSentenceMultiple: null,
+    multiCopyTimeoutMs: 2500,
+    toggleSecondarySub: null,
+    markAudioCard: null,
+    openRuntimeOptions: null,
+    openJimaku: null,
+    ...overrides,
+  };
+}
+
+test('registerOverlayShortcuts reports active overlay shortcuts when configured', () => {
+  assert.equal(
+    registerOverlayShortcuts(createShortcuts({ openJimaku: 'Ctrl+J' }), {
+      copySubtitle: () => {},
+      copySubtitleMultiple: () => {},
+      updateLastCardFromClipboard: () => {},
+      triggerFieldGrouping: () => {},
+      triggerSubsync: () => {},
+      mineSentence: () => {},
+      mineSentenceMultiple: () => {},
+      toggleSecondarySub: () => {},
+      markAudioCard: () => {},
+      openRuntimeOptions: () => {},
+      openJimaku: () => {},
+    }),
+    true,
+  );
+});
+
+test('registerOverlayShortcuts stays inactive when overlay shortcuts are absent', () => {
+  assert.equal(
+    registerOverlayShortcuts(createShortcuts(), {
+      copySubtitle: () => {},
+      copySubtitleMultiple: () => {},
+      updateLastCardFromClipboard: () => {},
+      triggerFieldGrouping: () => {},
+      triggerSubsync: () => {},
+      mineSentence: () => {},
+      mineSentenceMultiple: () => {},
+      toggleSecondarySub: () => {},
+      markAudioCard: () => {},
+      openRuntimeOptions: () => {},
+      openJimaku: () => {},
+    }),
+    false,
+  );
+});
+
+test('syncOverlayShortcutsRuntime deactivates cleanly when shortcuts were active', () => {
+  const calls: string[] = [];
+  const result = syncOverlayShortcutsRuntime(false, true, {
+    getConfiguredShortcuts: () => createShortcuts(),
+    getOverlayHandlers: () => ({
+      copySubtitle: () => {},
+      copySubtitleMultiple: () => {},
+      updateLastCardFromClipboard: () => {},
+      triggerFieldGrouping: () => {},
+      triggerSubsync: () => {},
+      mineSentence: () => {},
+      mineSentenceMultiple: () => {},
+      toggleSecondarySub: () => {},
+      markAudioCard: () => {},
+      openRuntimeOptions: () => {},
+      openJimaku: () => {},
+    }),
+    cancelPendingMultiCopy: () => {
+      calls.push('cancel-multi-copy');
+    },
+    cancelPendingMineSentenceMultiple: () => {
+      calls.push('cancel-mine-sentence-multiple');
+    },
+  });
+
+  assert.equal(result, false);
+  assert.deepEqual(calls, ['cancel-multi-copy', 'cancel-mine-sentence-multiple']);
+});

src/core/services/overlay-shortcut.ts

@@ -21,6 +21,27 @@ export interface OverlayShortcutLifecycleDeps {
   cancelPendingMineSentenceMultiple: () => void;
 }
 
+const OVERLAY_SHORTCUT_KEYS: Array<keyof Omit<ConfiguredShortcuts, 'multiCopyTimeoutMs'>> = [
+  'copySubtitle',
+  'copySubtitleMultiple',
+  'updateLastCardFromClipboard',
+  'triggerFieldGrouping',
+  'triggerSubsync',
+  'mineSentence',
+  'mineSentenceMultiple',
+  'toggleSecondarySub',
+  'markAudioCard',
+  'openRuntimeOptions',
+  'openJimaku',
+];
+
+function hasConfiguredOverlayShortcuts(shortcuts: ConfiguredShortcuts): boolean {
+  return OVERLAY_SHORTCUT_KEYS.some((key) => {
+    const shortcut = shortcuts[key];
+    return typeof shortcut === 'string' && shortcut.trim().length > 0;
+  });
+}
+
 export function shouldActivateOverlayShortcuts(args: {
   overlayRuntimeInitialized: boolean;
   isMacOSPlatform: boolean;
@@ -36,10 +57,10 @@ export function shouldActivateOverlayShortcuts(args: {
 }
 
 export function registerOverlayShortcuts(
-  _shortcuts: ConfiguredShortcuts,
+  shortcuts: ConfiguredShortcuts,
   _handlers: OverlayShortcutHandlers,
 ): boolean {
-  return false;
+  return hasConfiguredOverlayShortcuts(shortcuts);
 }
 
 export function unregisterOverlayShortcuts(_shortcuts: ConfiguredShortcuts): void {}

src/core/services/overlay-window.ts

@@ -17,6 +17,9 @@ const overlayWindowContentReady = new WeakSet<BrowserWindow>();
 const OVERLAY_WINDOW_CONTENT_READY_FLAG = '__subminerOverlayContentReady';
 
 export function isOverlayWindowContentReady(window: BrowserWindow): boolean {
+  if (window.isDestroyed()) {
+    return false;
+  }
   return (
     overlayWindowContentReady.has(window) ||
     (window as BrowserWindow & { [OVERLAY_WINDOW_CONTENT_READY_FLAG]?: boolean })[

src/core/services/session-bindings.test.ts

@@ -101,6 +101,55 @@ test('compileSessionBindings resolves CommandOrControl per platform', () => {
   assert.deepEqual(mac.bindings[0]?.key.modifiers, ['shift', 'meta']);
 });
 
+test('compileSessionBindings resolves CommandOrControl in DOM key strings per platform', () => {
+  const input = {
+    shortcuts: createShortcuts(),
+    keybindings: [createKeybinding('CommandOrControl+Shift+J', ['cycle', 'fullscreen'])],
+    statsToggleKey: 'CommandOrControl+Backquote',
+  };
+
+  const windows = compileSessionBindings({ ...input, platform: 'win32' });
+  const mac = compileSessionBindings({ ...input, platform: 'darwin' });
+
+  assert.deepEqual(
+    windows.bindings
+      .map((binding) => ({
+        sourcePath: binding.sourcePath,
+        modifiers: binding.key.modifiers,
+      }))
+      .sort((left, right) => left.sourcePath.localeCompare(right.sourcePath)),
+    [
+      {
+        sourcePath: 'keybindings[0].key',
+        modifiers: ['ctrl', 'shift'],
+      },
+      {
+        sourcePath: 'stats.toggleKey',
+        modifiers: ['ctrl'],
+      },
+    ],
+  );
+
+  assert.deepEqual(
+    mac.bindings
+      .map((binding) => ({
+        sourcePath: binding.sourcePath,
+        modifiers: binding.key.modifiers,
+      }))
+      .sort((left, right) => left.sourcePath.localeCompare(right.sourcePath)),
+    [
+      {
+        sourcePath: 'keybindings[0].key',
+        modifiers: ['shift', 'meta'],
+      },
+      {
+        sourcePath: 'stats.toggleKey',
+        modifiers: ['meta'],
+      },
+    ],
+  );
+});
+
 test('compileSessionBindings drops conflicting bindings that canonicalize to the same key', () => {
   const result = compileSessionBindings({
     shortcuts: createShortcuts({
@@ -173,3 +222,26 @@ test('compileSessionBindings warns on deprecated toggleVisibleOverlayGlobal conf
     },
   ]);
 });
+
+test('compileSessionBindings includes stats toggle in the shared session binding artifact', () => {
+  const result = compileSessionBindings({
+    shortcuts: createShortcuts(),
+    keybindings: [],
+    statsToggleKey: 'Backquote',
+    platform: 'win32',
+  });
+
+  assert.equal(result.warnings.length, 0);
+  assert.deepEqual(result.bindings, [
+    {
+      sourcePath: 'stats.toggleKey',
+      originalKey: 'Backquote',
+      key: {
+        code: 'Backquote',
+        modifiers: [],
+      },
+      actionType: 'session-action',
+      actionId: 'toggleStatsOverlay',
+    },
+  ]);
+});

src/core/services/session-bindings.ts

@@ -162,7 +162,10 @@ function parseAccelerator(
   };
 }
 
-function parseDomKeyString(key: string): { key: SessionKeySpec | null; message?: string } {
+function parseDomKeyString(
+  key: string,
+  platform: PlatformKeyModel,
+): { key: SessionKeySpec | null; message?: string } {
   const parts = key
     .split('+')
     .map((part) => part.trim())
@@ -194,7 +197,9 @@ function parseDomKeyString(key: string): { key: SessionKeySpec | null; message?:
       lower === 'cmd' ||
       lower === 'commandorcontrol'
     ) {
-      modifiers.push(lower === 'commandorcontrol' ? 'ctrl' : 'meta');
+      modifiers.push(
+        lower === 'commandorcontrol' ? (platform === 'darwin' ? 'meta' : 'ctrl') : 'meta',
+      );
       continue;
     }
     return {
@@ -335,7 +340,7 @@ export function compileSessionBindings(
   }
 
   if (statsToggleKey) {
-    const parsed = parseDomKeyString(statsToggleKey);
+    const parsed = parseDomKeyString(statsToggleKey, input.platform);
     if (!parsed.key) {
       warnings.push({
         kind: 'unsupported',
@@ -363,7 +368,7 @@ export function compileSessionBindings(
 
   input.keybindings.forEach((binding, index) => {
     if (!binding.command) return;
-    const parsed = parseDomKeyString(binding.key);
+    const parsed = parseDomKeyString(binding.key, input.platform);
     if (!parsed.key) {
       warnings.push({
         kind: 'unsupported',

src/renderer/yomitan-popup.test.ts

@@ -1,6 +1,11 @@
 import test from 'node:test';
 import assert from 'node:assert/strict';
-import { YOMITAN_LOOKUP_EVENT, registerYomitanLookupListener } from './yomitan-popup.js';
+import {
+  YOMITAN_LOOKUP_EVENT,
+  YOMITAN_POPUP_VISIBLE_HOST_SELECTOR,
+  isYomitanPopupVisible,
+  registerYomitanLookupListener,
+} from './yomitan-popup.js';
 
 test('registerYomitanLookupListener forwards the SubMiner Yomitan lookup event', () => {
   const target = new EventTarget();
@@ -16,3 +21,12 @@ test('registerYomitanLookupListener forwards the SubMiner Yomitan lookup event',
 
   assert.deepEqual(calls, ['lookup']);
 });
+
+test('isYomitanPopupVisible falls back to querySelector when querySelectorAll is unavailable', () => {
+  const root = {
+    querySelector: (selector: string) =>
+      selector === YOMITAN_POPUP_VISIBLE_HOST_SELECTOR ? ({} as Element) : null,
+  } as ParentNode;
+
+  assert.equal(isYomitanPopupVisible(root), true);
+});

src/renderer/yomitan-popup.ts

@@ -62,10 +62,14 @@ function queryPopupElements<T extends Element>(
   root: ParentNode | null | undefined,
   selector: string,
 ): T[] {
-  if (typeof root?.querySelectorAll !== 'function') {
-    return [];
+  if (typeof root?.querySelectorAll === 'function') {
+    return Array.from(root.querySelectorAll<T>(selector));
   }
-  return Array.from(root.querySelectorAll<T>(selector));
+  if (typeof root?.querySelector === 'function') {
+    const first = root.querySelector(selector) as T | null;
+    return first ? [first] : [];
+  }
+  return [];
 }
 
 export function isYomitanPopupVisible(root: ParentNode | null | undefined = document): boolean {