From 9ce5de2f2221a11e4706e8cc5a39897abf42d24f Mon Sep 17 00:00:00 2001 From: sudacode Date: Sat, 11 Apr 2026 15:49:18 -0700 Subject: [PATCH] fix: address PR #49 CodeRabbit review comments --- .github/workflows/prerelease.yml | 2 + ...ress-PR-49-CodeRabbit-review-follow-ups.md | 39 +++++++++-- plugin/subminer/environment.lua | 8 ++- src/core/services/app-ready.test.ts | 4 +- src/core/services/ipc.test.ts | 24 +++++++ src/core/services/session-bindings.test.ts | 19 ++++- src/core/services/session-bindings.ts | 2 +- src/core/services/startup.ts | 1 - src/main/overlay-runtime.test.ts | 47 +++++++++++-- .../runtime/first-run-setup-service.test.ts | 24 +++++++ src/main/runtime/first-run-setup-service.ts | 6 ++ src/renderer/handlers/keyboard.test.ts | 8 +++ src/renderer/handlers/keyboard.ts | 2 + src/renderer/renderer.ts | 4 +- src/shared/ipc/validators.ts | 4 ++ src/window-trackers/mpv-socket-match.test.ts | 56 +++++++++++++++ src/window-trackers/mpv-socket-match.ts | 40 +++++++++++ src/window-trackers/win32.ts | 70 +++++++++++++++++-- src/window-trackers/windows-tracker.ts | 3 +- 19 files changed, 337 insertions(+), 26 deletions(-) create mode 100644 src/window-trackers/mpv-socket-match.test.ts create mode 100644 src/window-trackers/mpv-socket-match.ts diff --git a/.github/workflows/prerelease.yml b/.github/workflows/prerelease.yml index 6d07ea96..daa76fd0 100644 --- a/.github/workflows/prerelease.yml +++ b/.github/workflows/prerelease.yml @@ -137,6 +137,7 @@ jobs: with: name: appimage path: release/*.AppImage + if-no-files-found: error build-macos: needs: [quality-gate] @@ -212,6 +213,7 @@ jobs: path: | release/*.dmg release/*.zip + if-no-files-found: error build-windows: needs: [quality-gate] diff --git a/backlog/tasks/task-286 - Assess-and-address-PR-49-CodeRabbit-review-follow-ups.md b/backlog/tasks/task-286 - Assess-and-address-PR-49-CodeRabbit-review-follow-ups.md index 9b293033..6f146ee1 100644 --- a/backlog/tasks/task-286 - Assess-and-address-PR-49-CodeRabbit-review-follow-ups.md +++ b/backlog/tasks/task-286 - Assess-and-address-PR-49-CodeRabbit-review-follow-ups.md @@ -1,9 +1,11 @@ --- id: TASK-286 title: 'Assess and address PR #49 CodeRabbit review follow-ups' -status: In Progress -assignee: [] +status: Done +assignee: + - codex created_date: '2026-04-11 18:55' +updated_date: '2026-04-11 22:40' labels: - bug - code-review @@ -28,7 +30,34 @@ Focus areas include the renderer mouse interaction fix, config hot-reload keyboa ## Acceptance Criteria -- [ ] #1 All actionable CodeRabbit comments on PR #49 are either fixed or shown to be obsolete with evidence. -- [ ] #2 Regression tests are added or updated for any behavior change that could regress. -- [ ] #3 The branch passes the repo's relevant verification checks for the touched areas. +- [x] #1 All actionable CodeRabbit comments on PR #49 are either fixed or shown to be obsolete with evidence. +- [x] #2 Regression tests are added or updated for any behavior change that could regress. +- [x] #3 The branch passes the repo's relevant verification checks for the touched areas. + +## Implementation Plan + + +1. Pull the current unresolved CodeRabbit review threads for PR #49 and cluster them into still-actionable fixes versus obsolete/nit-only items. +2. For each still-actionable behavior bug, add or extend the narrowest failing test first in the touched suite before changing production code. +3. Implement the minimal fixes across the affected runtime, renderer, plugin, IPC, and Windows tracker files, keeping each change traceable to the review thread. +4. Run targeted verification for the touched areas, update task notes with assessment results, and capture which review comments were fixed versus assessed as obsolete or deferred nitpicks. + + +## Implementation Notes + + +Assessed PR #49 CodeRabbit threads. Fixed the real regressions in first-run CLI gating, IPC session-action validation, renderer controller-modal lifecycle notifications, async subtitle-sidebar toggle guarding, plugin config-dir resolution priority, prerelease artifact upload failure handling, immersion tracker lazy startup, win32 z-order error handling, and Windows socket-aware mpv matching. + +Review assessment: the overlay-shortcut lifecycle comment is obsolete for the current architecture because overlay-local shortcuts are intentionally handled through the local fallback path and the runtime only tracks configured-state/cleanup. Refactor-only nit comments for splitting `scripts/build-changelog.ts` and `src/core/services/session-bindings.ts` were left as follow-up quality work, not behavior bugs in this PR round. + +Verification: `bun test src/main/runtime/first-run-setup-service.test.ts src/core/services/session-bindings.test.ts src/core/services/app-ready.test.ts src/core/services/ipc.test.ts src/renderer/handlers/keyboard.test.ts src/main/overlay-runtime.test.ts src/window-trackers/mpv-socket-match.test.ts`, `bun test src/window-trackers/windows-tracker.test.ts`, `bun run typecheck`, `lua scripts/test-plugin-lua-compat.lua`. + + +## Final Summary + + +Assessed the current CodeRabbit round on PR #49 and addressed the still-valid behavior issues rather than blanket-applying every bot suggestion. The branch now treats the new session/stats CLI flags as explicit startup commands during first-run setup, validates the new session actions through IPC, points session-binding command diagnostics at the correct config field, keeps immersion tracker startup lazy until later runtime triggers, and notifies overlay modal lifecycle state when controller-select/debug are opened from local keyboard bindings. I also switched the subtitle-sidebar IPC callback to the async guarded path so promise rejections feed renderer recovery instead of being dropped. + +On the Windows/plugin side, the mpv plugin now prefers config-file matches before falling back to an existing config directory, prerelease workflow uploads fail if expected Linux/macOS artifacts are missing, the Win32 z-order bind path now validates the `GetWindowLongW` call for the window above mpv, and the Windows tracker now passes the target socket path into native polling and filters mpv instances by command line so multiple sockets can be distinguished on Windows. Added/updated regression coverage for first-run gating, IPC validation, session-binding diagnostics, controller modal lifecycle notifications, modal ready-listener dispatch, and socket-path matching. Verification run: `bun run typecheck`, the targeted Bun test suites for the touched areas, `bun test src/window-trackers/windows-tracker.test.ts`, and `lua scripts/test-plugin-lua-compat.lua`. + diff --git a/plugin/subminer/environment.lua b/plugin/subminer/environment.lua index b6f6c2b6..df5a1f3e 100644 --- a/plugin/subminer/environment.lua +++ b/plugin/subminer/environment.lua @@ -71,7 +71,13 @@ function M.create(ctx) end for _, dir in ipairs(candidates) do - if file_exists(join_path(dir, "config.jsonc")) or file_exists(join_path(dir, "config.json")) or file_exists(dir) then + if file_exists(join_path(dir, "config.jsonc")) or file_exists(join_path(dir, "config.json")) then + return dir + end + end + + for _, dir in ipairs(candidates) do + if file_exists(dir) then return dir end end diff --git a/src/core/services/app-ready.test.ts b/src/core/services/app-ready.test.ts index e3f2a495..a98d359e 100644 --- a/src/core/services/app-ready.test.ts +++ b/src/core/services/app-ready.test.ts @@ -108,8 +108,8 @@ test('runAppReadyRuntime creates immersion tracker during heavy startup', async await runAppReadyRuntime(deps); - assert.ok(calls.includes('createImmersionTracker')); - assert.ok(calls.indexOf('createImmersionTracker') < calls.indexOf('handleInitialArgs')); + assert.equal(calls.includes('createImmersionTracker'), false); + assert.ok(calls.includes('log:Runtime ready: immersion tracker startup requested.')); }); test('runAppReadyRuntime keeps annotation websocket enabled when regular websocket auto-skips', async () => { diff --git a/src/core/services/ipc.test.ts b/src/core/services/ipc.test.ts index d8582d46..847607b6 100644 --- a/src/core/services/ipc.test.ts +++ b/src/core/services/ipc.test.ts @@ -897,6 +897,18 @@ test('registerIpcHandlers validates dispatchSessionAction payloads', async () => direction: -1, }, }); + await dispatchHandler!({}, { + actionId: 'toggleSubtitleSidebar', + }); + await dispatchHandler!({}, { + actionId: 'openSessionHelp', + }); + await dispatchHandler!({}, { + actionId: 'openControllerSelect', + }); + await dispatchHandler!({}, { + actionId: 'openControllerDebug', + }); assert.deepEqual(dispatched, [ { @@ -910,6 +922,18 @@ test('registerIpcHandlers validates dispatchSessionAction payloads', async () => direction: -1, }, }, + { + actionId: 'toggleSubtitleSidebar', + }, + { + actionId: 'openSessionHelp', + }, + { + actionId: 'openControllerSelect', + }, + { + actionId: 'openControllerDebug', + }, ]); }); diff --git a/src/core/services/session-bindings.test.ts b/src/core/services/session-bindings.test.ts index a6edd045..41f07251 100644 --- a/src/core/services/session-bindings.test.ts +++ b/src/core/services/session-bindings.test.ts @@ -226,7 +226,7 @@ test('compileSessionBindings rejects malformed command arrays', () => { assert.equal(result.bindings[0]?.actionType, 'mpv-command'); assert.deepEqual(result.bindings[0]?.command, ['show-text', 3000]); assert.deepEqual(result.warnings.map((warning) => `${warning.kind}:${warning.path}`), [ - 'unsupported:keybindings[1].key', + 'unsupported:keybindings[1].command', ]); }); @@ -242,8 +242,21 @@ test('compileSessionBindings rejects non-string command heads and extra args on assert.deepEqual(result.bindings, []); assert.deepEqual(result.warnings.map((warning) => `${warning.kind}:${warning.path}`), [ - 'unsupported:keybindings[0].key', - 'unsupported:keybindings[1].key', + 'unsupported:keybindings[0].command', + 'unsupported:keybindings[1].command', + ]); +}); + +test('compileSessionBindings points unsupported command warnings at the command field', () => { + const result = compileSessionBindings({ + shortcuts: createShortcuts(), + keybindings: [createKeybinding('Ctrl+K', [SPECIAL_COMMANDS.JIMAKU_OPEN, 'extra'] as never)], + platform: 'linux', + }); + + assert.deepEqual(result.bindings, []); + assert.deepEqual(result.warnings.map((warning) => `${warning.kind}:${warning.path}`), [ + 'unsupported:keybindings[0].command', ]); }); diff --git a/src/core/services/session-bindings.ts b/src/core/services/session-bindings.ts index 950f4e45..7b8272f4 100644 --- a/src/core/services/session-bindings.ts +++ b/src/core/services/session-bindings.ts @@ -436,7 +436,7 @@ export function compileSessionBindings( if (!resolved) { warnings.push({ kind: 'unsupported', - path: `keybindings[${index}].key`, + path: `keybindings[${index}].command`, value: binding.command, message: 'Unsupported keybinding command syntax.', }); diff --git a/src/core/services/startup.ts b/src/core/services/startup.ts index ae7033fe..bb563c1d 100644 --- a/src/core/services/startup.ts +++ b/src/core/services/startup.ts @@ -311,7 +311,6 @@ export async function runAppReadyRuntime(deps: AppReadyRuntimeDeps): Promise { - const callback = state.loadCallbacks.shift(); - callback?.(); + const callbacks = state.loadCallbacks.splice(0); + for (const callback of callbacks) { + callback(); + } }, emitReadyToShow: () => { - const callback = state.readyToShowCallbacks.shift(); - callback?.(); + const callbacks = state.readyToShowCallbacks.splice(0); + for (const callback of callbacks) { + callback(); + } }, once: (_event: 'ready-to-show', cb: () => void) => { state.readyToShowCallbacks.push(cb); @@ -624,6 +628,41 @@ test('sendToActiveOverlayWindow waits for modal ready-to-show before delivering assert.deepEqual(window.sent, [['runtime-options:open']]); }); +test('sendToActiveOverlayWindow flushes every queued load and ready listener before sending', () => { + const window = createMockWindow(); + window.contentReady = false; + const runtime = createOverlayModalRuntimeService({ + getMainWindow: () => null, + getModalWindow: () => window as never, + createModalWindow: () => { + throw new Error('modal window should not be created when already present'); + }, + getModalGeometry: () => ({ x: 0, y: 0, width: 400, height: 300 }), + setModalWindowBounds: () => {}, + }); + + assert.equal( + runtime.sendToActiveOverlayWindow('runtime-options:open', undefined, { + restoreOnModalClose: 'runtime-options', + }), + true, + ); + assert.equal( + runtime.sendToActiveOverlayWindow('session-help:open', undefined, { + restoreOnModalClose: 'session-help', + }), + true, + ); + assert.deepEqual(window.sent, []); + + window.emitDidFinishLoad(); + assert.deepEqual(window.sent, []); + + window.contentReady = true; + window.emitReadyToShow(); + assert.deepEqual(window.sent, [['runtime-options:open'], ['session-help:open']]); +}); + test('modal reopen creates a fresh window after close destroys the previous one', () => { const firstWindow = createMockWindow(); const secondWindow = createMockWindow(); diff --git a/src/main/runtime/first-run-setup-service.test.ts b/src/main/runtime/first-run-setup-service.test.ts index 95ed94aa..4111024b 100644 --- a/src/main/runtime/first-run-setup-service.test.ts +++ b/src/main/runtime/first-run-setup-service.test.ts @@ -104,6 +104,30 @@ test('shouldAutoOpenFirstRunSetup treats numeric startup counts as explicit comm ); }); +test('shouldAutoOpenFirstRunSetup treats session and stats startup commands as explicit commands', () => { + assert.equal( + shouldAutoOpenFirstRunSetup(makeArgs({ start: true, toggleSubtitleSidebar: true })), + false, + ); + assert.equal( + shouldAutoOpenFirstRunSetup(makeArgs({ background: true, openSessionHelp: true })), + false, + ); + assert.equal( + shouldAutoOpenFirstRunSetup(makeArgs({ start: true, openControllerSelect: true })), + false, + ); + assert.equal( + shouldAutoOpenFirstRunSetup(makeArgs({ background: true, openControllerDebug: true })), + false, + ); + assert.equal(shouldAutoOpenFirstRunSetup(makeArgs({ start: true, stats: true })), false); + assert.equal( + shouldAutoOpenFirstRunSetup(makeArgs({ background: true, jellyfinSubtitleUrlsOnly: true })), + false, + ); +}); + test('setup service auto-completes legacy installs with config and dictionaries', async () => { await withTempDir(async (root) => { const configDir = path.join(root, 'SubMiner'); diff --git a/src/main/runtime/first-run-setup-service.ts b/src/main/runtime/first-run-setup-service.ts index 908e90df..737c801b 100644 --- a/src/main/runtime/first-run-setup-service.ts +++ b/src/main/runtime/first-run-setup-service.ts @@ -79,7 +79,11 @@ function hasAnyStartupCommandBeyondSetup(args: CliArgs): boolean { args.triggerSubsync || args.markAudioCard || args.toggleStatsOverlay || + args.toggleSubtitleSidebar || args.openRuntimeOptions || + args.openSessionHelp || + args.openControllerSelect || + args.openControllerDebug || args.openJimaku || args.openYoutubePicker || args.openPlaylistBrowser || @@ -93,12 +97,14 @@ function hasAnyStartupCommandBeyondSetup(args: CliArgs): boolean { args.anilistSetup || args.anilistRetryQueue || args.dictionary || + args.stats || args.jellyfin || args.jellyfinLogin || args.jellyfinLogout || args.jellyfinLibraries || args.jellyfinItems || args.jellyfinSubtitles || + args.jellyfinSubtitleUrlsOnly || args.jellyfinPlay || args.jellyfinRemoteAnnounce || args.jellyfinPreviewAuth || diff --git a/src/renderer/handlers/keyboard.test.ts b/src/renderer/handlers/keyboard.test.ts index 4971e0c0..63999d80 100644 --- a/src/renderer/handlers/keyboard.test.ts +++ b/src/renderer/handlers/keyboard.test.ts @@ -78,6 +78,7 @@ function installKeyboardTestGlobals() { let markActiveVideoWatchedResult = true; let markActiveVideoWatchedCalls = 0; let statsToggleOverlayCalls = 0; + const openedModalNotifications: string[] = []; let selectionClearCount = 0; let selectionAddCount = 0; @@ -183,6 +184,9 @@ function installKeyboardTestGlobals() { focusMainWindowCalls += 1; return Promise.resolve(); }, + notifyOverlayModalOpened: (modal: string) => { + openedModalNotifications.push(modal); + }, }, }, }); @@ -312,6 +316,7 @@ function installKeyboardTestGlobals() { }, markActiveVideoWatchedCalls: () => markActiveVideoWatchedCalls, statsToggleOverlayCalls: () => statsToggleOverlayCalls, + openedModalNotifications, getPlaybackPaused: async () => playbackPausedResponse, setPlaybackPausedResponse: (value: boolean | null) => { playbackPausedResponse = value; @@ -806,6 +811,7 @@ test('keyboard mode: configured controller select binding opens locally without assert.equal(openControllerSelectCount(), 1); assert.deepEqual(testGlobals.sessionActions, []); + assert.deepEqual(testGlobals.openedModalNotifications, ['controller-select']); } finally { testGlobals.restore(); } @@ -835,6 +841,7 @@ test('keyboard mode: configured controller debug binding opens locally without d assert.equal(openControllerDebugCount(), 1); assert.deepEqual(testGlobals.sessionActions, []); + assert.deepEqual(testGlobals.openedModalNotifications, ['controller-debug']); } finally { testGlobals.restore(); } @@ -866,6 +873,7 @@ test('keyboard mode: configured controller debug binding is not swallowed while assert.equal(openControllerDebugCount(), 1); assert.deepEqual(testGlobals.sessionActions, []); + assert.deepEqual(testGlobals.openedModalNotifications, ['controller-debug']); } finally { testGlobals.restore(); } diff --git a/src/renderer/handlers/keyboard.ts b/src/renderer/handlers/keyboard.ts index eb161f96..ca99e31c 100644 --- a/src/renderer/handlers/keyboard.ts +++ b/src/renderer/handlers/keyboard.ts @@ -200,11 +200,13 @@ export function createKeyboardHandlers( } if (binding.actionType === 'session-action' && binding.actionId === 'openControllerSelect') { + window.electronAPI.notifyOverlayModalOpened('controller-select'); options.openControllerSelectModal?.(); return; } if (binding.actionType === 'session-action' && binding.actionId === 'openControllerDebug') { + window.electronAPI.notifyOverlayModalOpened('controller-debug'); options.openControllerDebugModal?.(); return; } diff --git a/src/renderer/renderer.ts b/src/renderer/renderer.ts index 699ddd72..2c828db6 100644 --- a/src/renderer/renderer.ts +++ b/src/renderer/renderer.ts @@ -510,8 +510,8 @@ function registerKeyboardCommandHandlers(): void { }); window.electronAPI.onSubtitleSidebarToggle(() => { - runGuarded('subtitle-sidebar:toggle', () => { - void subtitleSidebarModal.toggleSubtitleSidebarModal(); + runGuardedAsync('subtitle-sidebar:toggle', async () => { + await subtitleSidebarModal.toggleSubtitleSidebarModal(); }); }); } diff --git a/src/shared/ipc/validators.ts b/src/shared/ipc/validators.ts index c1bfa5f8..4fcb8c57 100644 --- a/src/shared/ipc/validators.ts +++ b/src/shared/ipc/validators.ts @@ -28,7 +28,11 @@ const SESSION_ACTION_IDS: SessionActionId[] = [ 'mineSentenceMultiple', 'toggleSecondarySub', 'markAudioCard', + 'toggleSubtitleSidebar', 'openRuntimeOptions', + 'openSessionHelp', + 'openControllerSelect', + 'openControllerDebug', 'openJimaku', 'openYoutubePicker', 'openPlaylistBrowser', diff --git a/src/window-trackers/mpv-socket-match.test.ts b/src/window-trackers/mpv-socket-match.test.ts new file mode 100644 index 00000000..83b11e99 --- /dev/null +++ b/src/window-trackers/mpv-socket-match.test.ts @@ -0,0 +1,56 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { filterMpvPollResultBySocketPath, matchesMpvSocketPathInCommandLine } from './mpv-socket-match'; +import type { MpvPollResult } from './win32'; + +function createPollResult(commandLines: Array): MpvPollResult { + return { + matches: commandLines.map((commandLine, index) => ({ + hwnd: index + 1, + bounds: { x: index * 10, y: 0, width: 1280, height: 720 }, + area: 1280 * 720, + isForeground: index === 0, + commandLine, + })), + focusState: true, + windowState: 'visible', + }; +} + +test('matchesMpvSocketPathInCommandLine accepts equals and space-delimited socket flags', () => { + assert.equal( + matchesMpvSocketPathInCommandLine( + 'mpv.exe --input-ipc-server=\\\\.\\pipe\\subminer-a video.mkv', + '\\\\.\\pipe\\subminer-a', + ), + true, + ); + assert.equal( + matchesMpvSocketPathInCommandLine( + 'mpv.exe --input-ipc-server "\\\\.\\pipe\\subminer-b" video.mkv', + '\\\\.\\pipe\\subminer-b', + ), + true, + ); + assert.equal( + matchesMpvSocketPathInCommandLine( + 'mpv.exe --input-ipc-server=\\\\.\\pipe\\subminer-a video.mkv', + '\\\\.\\pipe\\subminer-b', + ), + false, + ); +}); + +test('filterMpvPollResultBySocketPath keeps only matches for the requested socket path', () => { + const result = filterMpvPollResultBySocketPath( + createPollResult([ + 'mpv.exe --input-ipc-server=\\\\.\\pipe\\subminer-a video-a.mkv', + 'mpv.exe --input-ipc-server=\\\\.\\pipe\\subminer-b video-b.mkv', + null, + ]), + '\\\\.\\pipe\\subminer-b', + ); + + assert.deepEqual(result.matches.map((match) => match.hwnd), [2]); + assert.equal(result.windowState, 'visible'); +}); diff --git a/src/window-trackers/mpv-socket-match.ts b/src/window-trackers/mpv-socket-match.ts new file mode 100644 index 00000000..724b95c1 --- /dev/null +++ b/src/window-trackers/mpv-socket-match.ts @@ -0,0 +1,40 @@ +import type { MpvPollResult } from './win32'; + +function escapeRegex(text: string): string { + return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +export function matchesMpvSocketPathInCommandLine( + commandLine: string, + targetSocketPath: string, +): boolean { + if (!commandLine || !targetSocketPath) { + return false; + } + + const escapedSocketPath = escapeRegex(targetSocketPath); + return new RegExp(`--input-ipc-server(?:=|\\s+)("?${escapedSocketPath}"?)`, 'i').test( + commandLine, + ); +} + +export function filterMpvPollResultBySocketPath( + result: MpvPollResult, + targetSocketPath?: string | null, +): MpvPollResult { + if (!targetSocketPath) { + return result; + } + + const matches = result.matches.filter( + (match) => + typeof match.commandLine === 'string' && + matchesMpvSocketPathInCommandLine(match.commandLine, targetSocketPath), + ); + + return { + matches, + focusState: matches.some((match) => match.isForeground), + windowState: matches.length > 0 ? 'visible' : 'not-found', + }; +} diff --git a/src/window-trackers/win32.ts b/src/window-trackers/win32.ts index e3739565..bde62f0e 100644 --- a/src/window-trackers/win32.ts +++ b/src/window-trackers/win32.ts @@ -1,4 +1,6 @@ +import { execFileSync } from 'node:child_process'; import koffi from 'koffi'; +import { matchesMpvSocketPathInCommandLine } from './mpv-socket-match'; const user32 = koffi.load('user32.dll'); const dwmapi = koffi.load('dwmapi.dll'); @@ -126,6 +128,7 @@ export interface MpvWindowMatch { bounds: WindowBounds; area: number; isForeground: boolean; + commandLine?: string | null; } export interface MpvPollResult { @@ -170,12 +173,48 @@ function getProcessNameByPid(pid: number): string | null { } } -export function findMpvWindows(): MpvPollResult { +const processCommandLineCache = new Map(); + +function getProcessCommandLineByPid(pid: number): string | null { + if (processCommandLineCache.has(pid)) { + return processCommandLineCache.get(pid) ?? null; + } + + let commandLine: string | null = null; + try { + const output = execFileSync( + 'powershell.exe', + [ + '-NoProfile', + '-NonInteractive', + '-ExecutionPolicy', + 'Bypass', + '-Command', + `$process = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; if ($process -and $process.CommandLine) { [Console]::Out.Write($process.CommandLine) }`, + ], + { + encoding: 'utf8', + windowsHide: true, + stdio: ['ignore', 'pipe', 'ignore'], + timeout: 1500, + }, + ).trim(); + commandLine = output.length > 0 ? output : null; + } catch { + commandLine = null; + } + + processCommandLineCache.set(pid, commandLine); + return commandLine; +} + +export function findMpvWindows(targetSocketPath?: string | null): MpvPollResult { const foregroundHwnd = GetForegroundWindow(); const matches: MpvWindowMatch[] = []; let hasMinimized = false; let hasFocused = false; const processNameCache = new Map(); + const processCommandLineLookupCache = new Map(); const cb = koffi.register((hwnd: number, _lParam: number) => { if (!IsWindowVisible(hwnd)) return true; @@ -193,6 +232,18 @@ export function findMpvWindows(): MpvPollResult { if (!processName || processName.toLowerCase() !== 'mpv') return true; + let commandLine: string | null = null; + if (targetSocketPath) { + commandLine = processCommandLineLookupCache.get(pidValue) ?? null; + if (!processCommandLineLookupCache.has(pidValue)) { + commandLine = getProcessCommandLineByPid(pidValue); + processCommandLineLookupCache.set(pidValue, commandLine); + } + if (!commandLine || !matchesMpvSocketPathInCommandLine(commandLine, targetSocketPath)) { + return true; + } + } + if (IsIconic(hwnd)) { hasMinimized = true; return true; @@ -209,6 +260,7 @@ export function findMpvWindows(): MpvPollResult { bounds, area: bounds.width * bounds.height, isForeground, + commandLine, }); return true; @@ -290,10 +342,18 @@ export function bindOverlayAboveMpv(overlayHwnd: number, mpvHwnd: number): void let insertAfter = HWND_TOP; if (windowAboveMpv !== 0) { - const aboveExStyle = GetWindowLongW(windowAboveMpv, GWL_EXSTYLE); - const aboveIsTopmost = (aboveExStyle & WS_EX_TOPMOST) !== 0; - if (aboveIsTopmost === mpvIsTopmost) { - insertAfter = windowAboveMpv; + try { + resetLastError(); + const aboveExStyle = assertGetWindowLongSucceeded( + 'bindOverlayAboveMpv window above style', + GetWindowLongW(windowAboveMpv, GWL_EXSTYLE), + ); + const aboveIsTopmost = (aboveExStyle & WS_EX_TOPMOST) !== 0; + if (aboveIsTopmost === mpvIsTopmost) { + insertAfter = windowAboveMpv; + } + } catch { + insertAfter = HWND_TOP; } } diff --git a/src/window-trackers/windows-tracker.ts b/src/window-trackers/windows-tracker.ts index 14c8cb86..4152aaa6 100644 --- a/src/window-trackers/windows-tracker.ts +++ b/src/window-trackers/windows-tracker.ts @@ -32,9 +32,8 @@ type WindowsTrackerDeps = { }; function defaultPollMpvWindows(_targetMpvSocketPath?: string | null): MpvPollResult { - void _targetMpvSocketPath; const win32 = require('./win32') as typeof import('./win32'); - return win32.findMpvWindows(); + return win32.findMpvWindows(_targetMpvSocketPath); } export class WindowsWindowTracker extends BaseWindowTracker {