fix: address CodeRabbit review and ci

This commit is contained in:
2026-04-10 19:11:16 -07:00
parent aa6903d457
commit d81fe87982
14 changed files with 239 additions and 43 deletions

View File

@@ -1,14 +1,13 @@
import type { BrowserWindow } from 'electron';
import { BaseWindowTracker } from '../../window-trackers';
import { WindowGeometry } from '../../types';
import { OVERLAY_WINDOW_CONTENT_READY_FLAG } from './overlay-window-flags';
const WINDOWS_OVERLAY_REVEAL_DELAY_MS = 48;
const pendingWindowsOverlayRevealTimeoutByWindow = new WeakMap<
BrowserWindow,
ReturnType<typeof setTimeout>
>();
const OVERLAY_WINDOW_CONTENT_READY_FLAG = '__subminerOverlayContentReady';
function setOverlayWindowOpacity(window: BrowserWindow, opacity: number): void {
const opacityCapableWindow = window as BrowserWindow & {
setOpacity?: (opacity: number) => void;

View File

@@ -0,0 +1 @@
export const OVERLAY_WINDOW_CONTENT_READY_FLAG = '__subminerOverlayContentReady';

View File

@@ -10,11 +10,12 @@ import {
} from './overlay-window-input';
import { buildOverlayWindowOptions } from './overlay-window-options';
import { normalizeOverlayWindowBoundsForPlatform } from './overlay-window-bounds';
import { OVERLAY_WINDOW_CONTENT_READY_FLAG } from './overlay-window-flags';
export { OVERLAY_WINDOW_CONTENT_READY_FLAG } from './overlay-window-flags';
const logger = createLogger('main:overlay-window');
const overlayWindowLayerByInstance = new WeakMap<BrowserWindow, OverlayWindowKind>();
const overlayWindowContentReady = new WeakSet<BrowserWindow>();
const OVERLAY_WINDOW_CONTENT_READY_FLAG = '__subminerOverlayContentReady';
export function isOverlayWindowContentReady(window: BrowserWindow): boolean {
if (window.isDestroyed()) {

View File

@@ -137,9 +137,9 @@ import {
clearWindowsOverlayOwnerNative,
ensureWindowsOverlayTransparencyNative,
getWindowsForegroundProcessNameNative,
queryWindowsForegroundProcessName,
queryWindowsTargetWindowHandle,
setWindowsOverlayOwnerNative,
shouldUseWindowsTrackerPowershellFallback,
} from './window-trackers/windows-helper';
import {
commandNeedsOverlayStartupPrereqs,
@@ -453,6 +453,7 @@ import { handleCliCommandRuntimeServiceWithContext } from './main/cli-runtime';
import { createOverlayModalRuntimeService } from './main/overlay-runtime';
import { createOverlayModalInputState } from './main/runtime/overlay-modal-input-state';
import { openYoutubeTrackPicker } from './main/runtime/youtube-picker-open';
import { openRuntimeOptionsModal as openRuntimeOptionsModalRuntime } from './main/runtime/runtime-options-open';
import { createPlaylistBrowserIpcRuntime } from './main/runtime/playlist-browser-ipc';
import { writeSessionBindingsArtifact } from './main/runtime/session-bindings-artifact';
import { openOverlayHostedModal } from './main/runtime/overlay-hosted-modal-open';
@@ -1903,7 +1904,6 @@ let windowsVisibleOverlayZOrderRetryTimeouts: Array<ReturnType<typeof setTimeout
let windowsVisibleOverlayZOrderSyncInFlight = false;
let windowsVisibleOverlayZOrderSyncQueued = false;
let windowsVisibleOverlayForegroundPollInterval: ReturnType<typeof setInterval> | null = null;
let windowsVisibleOverlayForegroundPollInFlight = false;
let lastWindowsVisibleOverlayForegroundProcessName: string | null = null;
let lastWindowsVisibleOverlayBlurredAtMs = 0;
@@ -1940,9 +1940,11 @@ function resolveWindowsOverlayBindTargetHandle(targetMpvSocketPath?: string | nu
return null;
}
const helperTargetHwnd = queryWindowsTargetWindowHandle({ targetMpvSocketPath });
if (helperTargetHwnd !== null) {
return helperTargetHwnd;
if (shouldUseWindowsTrackerPowershellFallback()) {
const helperTargetHwnd = queryWindowsTargetWindowHandle({ targetMpvSocketPath });
if (helperTargetHwnd !== null) {
return helperTargetHwnd;
}
}
try {
@@ -2103,6 +2105,15 @@ function ensureWindowsVisibleOverlayForegroundPollLoop(): void {
}, WINDOWS_VISIBLE_OVERLAY_FOREGROUND_POLL_INTERVAL_MS);
}
function clearWindowsVisibleOverlayForegroundPollLoop(): void {
if (windowsVisibleOverlayForegroundPollInterval === null) {
return;
}
clearInterval(windowsVisibleOverlayForegroundPollInterval);
windowsVisibleOverlayForegroundPollInterval = null;
}
function scheduleVisibleOverlayBlurRefresh(): void {
if (process.platform !== 'win32') {
return;
@@ -2211,23 +2222,19 @@ function setOverlayDebugVisualizationEnabled(enabled: boolean): void {
}
function openRuntimeOptionsPalette(): void {
const opened = openOverlayHostedModal(
{
ensureOverlayStartupPrereqs: () => ensureOverlayStartupPrereqs(),
ensureOverlayWindowsReadyForVisibilityActions: () =>
ensureOverlayWindowsReadyForVisibilityActions(),
sendToActiveOverlayWindow: (channel, payload, runtimeOptions) =>
sendToActiveOverlayWindow(channel, payload, runtimeOptions),
},
{
channel: IPC_CHANNELS.event.runtimeOptionsOpen,
modal: 'runtime-options',
preferModalWindow: true,
},
);
if (!opened) {
showMpvOsd('Runtime options overlay unavailable.');
}
void openRuntimeOptionsModalRuntime({
ensureOverlayStartupPrereqs: () => ensureOverlayStartupPrereqs(),
ensureOverlayWindowsReadyForVisibilityActions: () =>
ensureOverlayWindowsReadyForVisibilityActions(),
sendToActiveOverlayWindow: (channel, payload, runtimeOptions) =>
sendToActiveOverlayWindow(channel, payload, runtimeOptions),
waitForModalOpen: (modal, timeoutMs) => overlayModalRuntime.waitForModalOpen(modal, timeoutMs),
logWarn: (message) => logger.warn(message),
}).then((opened) => {
if (!opened) {
showMpvOsd('Runtime options overlay unavailable.');
}
});
}
function openJimakuOverlay(): void {
@@ -3034,6 +3041,8 @@ const {
annotationSubtitleWsService.stop();
},
stopTexthookerService: () => texthookerService.stop(),
clearWindowsVisibleOverlayForegroundPollLoop: () =>
clearWindowsVisibleOverlayForegroundPollLoop(),
getMainOverlayWindow: () => overlayManager.getMainWindow(),
clearMainOverlayWindow: () => overlayManager.setMainWindow(null),
getModalOverlayWindow: () => overlayManager.getModalWindow(),

View File

@@ -492,7 +492,7 @@ test('notifyOverlayModalOpened enables input on visible main overlay window when
assert.equal(mainWindow.webContentsFocused, true);
});
test('handleOverlayModalClosed resets modal state even when modal window does not exist', () => {
test('handleOverlayModalClosed is a no-op when no modal window can be targeted', () => {
const state: boolean[] = [];
const runtime = createOverlayModalRuntimeService(
{
@@ -509,13 +509,14 @@ test('handleOverlayModalClosed resets modal state even when modal window does no
},
);
runtime.sendToActiveOverlayWindow('runtime-options:open', undefined, {
const sent = runtime.sendToActiveOverlayWindow('runtime-options:open', undefined, {
restoreOnModalClose: 'runtime-options',
});
assert.equal(sent, false);
runtime.notifyOverlayModalOpened('runtime-options');
runtime.handleOverlayModalClosed('runtime-options');
assert.deepEqual(state, [true, false]);
assert.deepEqual(state, []);
});
test('handleOverlayModalClosed hides modal window for single kiku modal', () => {
@@ -637,10 +638,11 @@ test('warm modal window reopen becomes interactive immediately on the second ope
});
test('waitForModalOpen resolves true after modal acknowledgement', async () => {
const modalWindow = createMockWindow();
const runtime = createOverlayModalRuntimeService({
getMainWindow: () => null,
getModalWindow: () => null,
createModalWindow: () => null,
getModalWindow: () => modalWindow as never,
createModalWindow: () => modalWindow as never,
getModalGeometry: () => ({ x: 0, y: 0, width: 400, height: 300 }),
setModalWindowBounds: () => {},
});

View File

@@ -1,9 +1,9 @@
import type { BrowserWindow } from 'electron';
import type { OverlayHostedModal } from '../shared/ipc/contracts';
import type { WindowGeometry } from '../types';
import { OVERLAY_WINDOW_CONTENT_READY_FLAG } from '../core/services/overlay-window-flags';
const MODAL_REVEAL_FALLBACK_DELAY_MS = 250;
const OVERLAY_WINDOW_CONTENT_READY_FLAG = '__subminerOverlayContentReady';
export interface OverlayWindowResolver {
getMainWindow: () => BrowserWindow | null;

View File

@@ -18,6 +18,8 @@ test('cleanup deps builder returns handlers that guard optional runtime objects'
unregisterAllGlobalShortcuts: () => calls.push('unregister-shortcuts'),
stopSubtitleWebsocket: () => calls.push('stop-ws'),
stopTexthookerService: () => calls.push('stop-texthooker'),
clearWindowsVisibleOverlayForegroundPollLoop: () =>
calls.push('clear-windows-visible-overlay-foreground-poll-loop'),
getMainOverlayWindow: () => ({
isDestroyed: () => false,
destroy: () => calls.push('destroy-main-overlay-window'),
@@ -99,6 +101,7 @@ test('cleanup deps builder skips destroyed yomitan window', () => {
unregisterAllGlobalShortcuts: () => {},
stopSubtitleWebsocket: () => {},
stopTexthookerService: () => {},
clearWindowsVisibleOverlayForegroundPollLoop: () => {},
getMainOverlayWindow: () => ({
isDestroyed: () => true,
destroy: () => calls.push('destroy-main-overlay-window'),

View File

@@ -25,6 +25,7 @@ export function createBuildOnWillQuitCleanupDepsHandler(deps: {
unregisterAllGlobalShortcuts: () => void;
stopSubtitleWebsocket: () => void;
stopTexthookerService: () => void;
clearWindowsVisibleOverlayForegroundPollLoop: () => void;
getMainOverlayWindow: () => DestroyableWindow | null;
clearMainOverlayWindow: () => void;
getModalOverlayWindow: () => DestroyableWindow | null;
@@ -64,6 +65,8 @@ export function createBuildOnWillQuitCleanupDepsHandler(deps: {
unregisterAllGlobalShortcuts: () => deps.unregisterAllGlobalShortcuts(),
stopSubtitleWebsocket: () => deps.stopSubtitleWebsocket(),
stopTexthookerService: () => deps.stopTexthookerService(),
clearWindowsVisibleOverlayForegroundPollLoop: () =>
deps.clearWindowsVisibleOverlayForegroundPollLoop(),
destroyMainOverlayWindow: () => {
const window = deps.getMainOverlayWindow();
if (!window) return;

View File

@@ -21,6 +21,7 @@ test('composeStartupLifecycleHandlers returns callable startup lifecycle handler
unregisterAllGlobalShortcuts: () => {},
stopSubtitleWebsocket: () => {},
stopTexthookerService: () => {},
clearWindowsVisibleOverlayForegroundPollLoop: () => {},
getMainOverlayWindow: () => null,
clearMainOverlayWindow: () => {},
getModalOverlayWindow: () => null,

View File

@@ -0,0 +1,88 @@
import assert from 'node:assert/strict';
import test from 'node:test';
import { openRuntimeOptionsModal } from './runtime-options-open';
test('runtime options open prefers dedicated modal window on first attempt', async () => {
const calls: string[] = [];
const opened = await openRuntimeOptionsModal({
ensureOverlayStartupPrereqs: () => {
calls.push('ensure-startup');
},
ensureOverlayWindowsReadyForVisibilityActions: () => {
calls.push('ensure-windows');
},
sendToActiveOverlayWindow: (channel, payload, options) => {
calls.push(`send:${channel}`);
assert.equal(payload, undefined);
assert.deepEqual(options, {
restoreOnModalClose: 'runtime-options',
preferModalWindow: true,
});
return true;
},
waitForModalOpen: async () => true,
logWarn: () => {
throw new Error('should not warn on first-attempt success');
},
});
assert.equal(opened, true);
assert.deepEqual(calls, ['ensure-startup', 'ensure-windows', 'send:runtime-options:open']);
});
test('runtime options open retries after an open timeout', async () => {
const calls: string[] = [];
const warnings: string[] = [];
let waitCalls = 0;
const opened = await openRuntimeOptionsModal({
ensureOverlayStartupPrereqs: () => {
calls.push('ensure-startup');
},
ensureOverlayWindowsReadyForVisibilityActions: () => {
calls.push('ensure-windows');
},
sendToActiveOverlayWindow: (channel, payload, options) => {
calls.push(`send:${channel}`);
assert.equal(payload, undefined);
assert.deepEqual(options, {
restoreOnModalClose: 'runtime-options',
preferModalWindow: true,
});
return true;
},
waitForModalOpen: async () => {
waitCalls += 1;
return waitCalls === 2;
},
logWarn: (message) => {
warnings.push(message);
},
});
assert.equal(opened, true);
assert.deepEqual(calls, [
'ensure-startup',
'ensure-windows',
'send:runtime-options:open',
'ensure-startup',
'ensure-windows',
'send:runtime-options:open',
]);
assert.deepEqual(warnings, [
'Runtime options modal did not acknowledge modal open on first attempt; retrying dedicated modal window.',
]);
});
test('runtime options open fails when no overlay window can be targeted', async () => {
const opened = await openRuntimeOptionsModal({
ensureOverlayStartupPrereqs: () => {},
ensureOverlayWindowsReadyForVisibilityActions: () => {},
sendToActiveOverlayWindow: () => false,
waitForModalOpen: async () => true,
logWarn: () => {},
});
assert.equal(opened, false);
});

View File

@@ -0,0 +1,45 @@
import type { OverlayHostedModal } from '../../shared/ipc/contracts';
const RUNTIME_OPTIONS_MODAL: OverlayHostedModal = 'runtime-options';
const RUNTIME_OPTIONS_OPEN_TIMEOUT_MS = 1500;
export async function openRuntimeOptionsModal(deps: {
ensureOverlayStartupPrereqs: () => void;
ensureOverlayWindowsReadyForVisibilityActions: () => void;
sendToActiveOverlayWindow: (
channel: string,
payload?: unknown,
runtimeOptions?: {
restoreOnModalClose?: OverlayHostedModal;
preferModalWindow?: boolean;
},
) => boolean;
waitForModalOpen: (modal: OverlayHostedModal, timeoutMs: number) => Promise<boolean>;
logWarn: (message: string) => void;
}): Promise<boolean> {
const sendOpen = (): boolean => {
deps.ensureOverlayStartupPrereqs();
deps.ensureOverlayWindowsReadyForVisibilityActions();
return deps.sendToActiveOverlayWindow('runtime-options:open', undefined, {
restoreOnModalClose: RUNTIME_OPTIONS_MODAL,
preferModalWindow: true,
});
};
if (!sendOpen()) {
return false;
}
if (await deps.waitForModalOpen(RUNTIME_OPTIONS_MODAL, RUNTIME_OPTIONS_OPEN_TIMEOUT_MS)) {
return true;
}
deps.logWarn(
'Runtime options modal did not acknowledge modal open on first attempt; retrying dedicated modal window.',
);
if (!sendOpen()) {
return false;
}
return await deps.waitForModalOpen(RUNTIME_OPTIONS_MODAL, RUNTIME_OPTIONS_OPEN_TIMEOUT_MS);
}

View File

@@ -1,5 +1,6 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import * as windowsHelper from './windows-helper';
import {
lowerWindowsOverlayInZOrder,
parseWindowTrackerHelperForegroundProcess,
@@ -204,6 +205,41 @@ test('queryWindowsTargetWindowHandle resolves the selected hwnd from the powersh
assert.deepEqual(capturedArgs, ['\\\\.\\pipe\\subminer-socket']);
});
test('shouldUseWindowsTrackerPowershellFallback returns true for explicit powershell mode', () => {
assert.equal(
windowsHelper.shouldUseWindowsTrackerPowershellFallback({
helperModeEnv: 'powershell',
}),
true,
);
});
test('shouldUseWindowsTrackerPowershellFallback returns true for ps1 helper path override', () => {
assert.equal(
windowsHelper.shouldUseWindowsTrackerPowershellFallback({
helperPathEnv: 'C:\\custom\\tracker.ps1',
}),
true,
);
});
test('shouldUseWindowsTrackerPowershellFallback returns false for default and native modes', () => {
assert.equal(
windowsHelper.shouldUseWindowsTrackerPowershellFallback({
helperModeEnv: 'auto',
helperPathEnv: undefined,
}),
false,
);
assert.equal(
windowsHelper.shouldUseWindowsTrackerPowershellFallback({
helperModeEnv: 'native',
helperPathEnv: 'C:\\custom\\tracker.exe',
}),
false,
);
});
test('resolveWindowsTrackerHelper auto mode prefers native helper when present', () => {
const helper = resolveWindowsTrackerHelper({
dirname: 'C:\\repo\\dist\\window-trackers',

View File

@@ -64,6 +64,21 @@ function normalizeHelperMode(value: string | undefined): WindowsTrackerHelperMod
return 'auto';
}
export function shouldUseWindowsTrackerPowershellFallback(options: {
helperModeEnv?: string | undefined;
helperPathEnv?: string | undefined;
} = {}): boolean {
const mode = normalizeHelperMode(
options.helperModeEnv ?? process.env.SUBMINER_WINDOWS_TRACKER_HELPER,
);
if (mode === 'powershell') {
return true;
}
const helperPath = options.helperPathEnv ?? process.env.SUBMINER_WINDOWS_TRACKER_HELPER_PATH;
return helperPath?.trim().toLowerCase().endsWith('.ps1') ?? false;
}
function inferHelperKindFromPath(helperPath: string): WindowsTrackerHelperKind | null {
const normalized = helperPath.trim().toLowerCase();
if (normalized.endsWith('.exe')) return 'native';

View File

@@ -19,7 +19,10 @@
import { BaseWindowTracker } from './base-tracker';
import type { WindowGeometry } from '../types';
import type { MpvPollResult } from './win32';
import { queryWindowsTrackerMpvWindows } from './windows-helper';
import {
queryWindowsTrackerMpvWindows,
shouldUseWindowsTrackerPowershellFallback,
} from './windows-helper';
import { createLogger } from '../logger';
const log = createLogger('tracker').child('windows');
@@ -32,18 +35,8 @@ type WindowsTrackerDeps = {
now?: () => number;
};
function shouldUsePowershellTrackerFallback(): boolean {
const helperMode = process.env.SUBMINER_WINDOWS_TRACKER_HELPER?.trim().toLowerCase();
if (helperMode === 'powershell') {
return true;
}
const helperPath = process.env.SUBMINER_WINDOWS_TRACKER_HELPER_PATH?.trim().toLowerCase();
return helperPath?.endsWith('.ps1') ?? false;
}
function defaultPollMpvWindows(targetMpvSocketPath?: string | null): MpvPollResult {
if (targetMpvSocketPath && shouldUsePowershellTrackerFallback()) {
if (targetMpvSocketPath && shouldUseWindowsTrackerPowershellFallback()) {
const helperResult = queryWindowsTrackerMpvWindows({
targetMpvSocketPath,
});