Address CodeRabbit follow-ups for PR 40

This commit is contained in:
2026-04-03 11:37:12 -07:00
parent aa0385904e
commit c664f9f605
7 changed files with 195 additions and 10 deletions

View File

@@ -6,6 +6,8 @@
- AniList: Stopped post-watch tracking from sending a second progress update when the current episode was already satisfied by a ready retry item in the same watch-completion pass.
- Playback: Fixed managed local playback so duplicate startup-ready retries no longer unpause media after a later manual pause on the same file.
- Playback: Fixed managed local subtitle auto-selection so local files reuse configured primary/secondary subtitle language priorities instead of staying on mpv's initial `sid=auto` guess.
- Launcher: Hardened `--launch-mpv` parsing and Windows binary resolution so valueless flags do not swallow media targets and symlinked launcher installs do not short-circuit PATH lookup.
- Playback: Prevented stale async playlist-browser subtitle rearm callbacks from overriding newer subtitle selections during rapid file changes.
## v0.10.0 (2026-03-29)

View File

@@ -460,6 +460,18 @@ function withAccessSyncStub(
}
}
function withRealpathSyncStub(resolvePath: (filePath: string) => string, run: () => void): void {
const originalRealpathSync = fs.realpathSync;
try {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(fs as any).realpathSync = (filePath: string): string => resolvePath(filePath);
run();
} finally {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(fs as any).realpathSync = originalRealpathSync;
}
}
test('findAppBinary resolves ~/.local/bin/SubMiner.AppImage when it exists', { concurrency: false }, () => {
const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-test-home-'));
const originalHomedir = os.homedir;
@@ -527,6 +539,44 @@ test('findAppBinary finds subminer on PATH when AppImage candidates do not exist
}
});
test('findAppBinary excludes PATH matches that canonicalize to the launcher path', { concurrency: false }, () => {
const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-test-realpath-'));
const originalHomedir = os.homedir;
const originalPath = process.env.PATH;
try {
os.homedir = () => baseDir;
const binDir = path.join(baseDir, 'bin');
const wrapperPath = path.join(binDir, 'subminer');
const canonicalPath = path.join(baseDir, 'launch', 'subminer');
makeExecutable(wrapperPath);
process.env.PATH = `${binDir}${path.delimiter}${originalPath ?? ''}`;
withFindAppBinaryPlatformSandbox('linux', (pathModule) => {
withAccessSyncStub(
(filePath) => filePath === wrapperPath,
() => {
withRealpathSyncStub(
(filePath) => {
if (filePath === canonicalPath || filePath === wrapperPath) {
return canonicalPath;
}
return filePath;
},
() => {
const result = findAppBinary(canonicalPath, pathModule);
assert.equal(result, null);
},
);
},
);
});
} finally {
os.homedir = originalHomedir;
process.env.PATH = originalPath;
fs.rmSync(baseDir, { recursive: true, force: true });
}
});
test('findAppBinary resolves Windows install paths when present', { concurrency: false }, () => {
const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-test-win-'));
const originalHomedir = os.homedir;

View File

@@ -18,6 +18,7 @@ import {
uniqueNormalizedLangCodes,
sleep,
normalizeLangCode,
realpathMaybe,
} from './util.js';
export const state = {
@@ -380,8 +381,8 @@ export function findAppBinary(selfPath: string, pathModule: PathModule = path):
);
if (fromPath) {
const resolvedSelf = pathModule.resolve(selfPath);
const resolvedCandidate = pathModule.resolve(fromPath);
const resolvedSelf = realpathMaybe(selfPath);
const resolvedCandidate = realpathMaybe(fromPath);
if (resolvedSelf !== resolvedCandidate) return fromPath;
}

View File

@@ -113,6 +113,15 @@ test('launch-mpv entry helpers detect and normalize targets', () => {
]),
['--input-ipc-server', '\\\\.\\pipe\\custom-subminer-socket', '--alang', 'ja,jpn'],
);
assert.deepEqual(
normalizeLaunchMpvExtraArgs([
'SubMiner.exe',
'--launch-mpv',
'--fullscreen',
'C:\\a.mkv',
]),
['--fullscreen'],
);
assert.deepEqual(
normalizeLaunchMpvTargets([
'SubMiner.exe',
@@ -126,6 +135,20 @@ test('launch-mpv entry helpers detect and normalize targets', () => {
]),
['C:\\a.mkv', 'C:\\b.mkv'],
);
assert.deepEqual(
normalizeLaunchMpvTargets(['SubMiner.exe', '--launch-mpv', '--fullscreen', 'C:\\a.mkv']),
['C:\\a.mkv'],
);
assert.deepEqual(
normalizeLaunchMpvExtraArgs([
'SubMiner.exe',
'--launch-mpv',
'--msg-level',
'all=warn',
'C:\\a.mkv',
]),
['--msg-level', 'all=warn'],
);
});
test('stats-daemon entry helper detects internal daemon commands', () => {

View File

@@ -8,6 +8,23 @@ const START_ARG = '--start';
const PASSWORD_STORE_ARG = '--password-store';
const BACKGROUND_CHILD_ENV = 'SUBMINER_BACKGROUND_CHILD';
const APP_NAME = 'SubMiner';
const MPV_LONG_OPTIONS_WITH_SEPARATE_VALUES = new Set([
'--alang',
'--audio-file',
'--input-ipc-server',
'--log-file',
'--msg-level',
'--profile',
'--script',
'--script-opts',
'--scripts',
'--slang',
'--sub-file',
'--sub-file-paths',
'--title',
'--volume',
'--ytdl-format',
]);
type EarlyAppLike = {
setName: (name: string) => void;
@@ -53,6 +70,15 @@ function removePassiveStartupArgs(argv: string[]): string[] {
return filtered;
}
function consumesLaunchMpvValue(token: string): boolean {
return (
token.startsWith('--') &&
token !== '--' &&
!token.includes('=') &&
MPV_LONG_OPTIONS_WITH_SEPARATE_VALUES.has(token)
);
}
function parseCliArgs(argv: string[]): CliArgs {
return parseArgs(argv);
}
@@ -144,7 +170,7 @@ export function normalizeLaunchMpvTargets(argv: string[]): string[] {
}
if (token.startsWith('--')) {
if (!token.includes('=') && i + 1 < argv.length) {
if (consumesLaunchMpvValue(token) && i + 1 < argv.length) {
const value = argv[i + 1];
if (value && !value.startsWith('-')) {
i += 1;
@@ -179,7 +205,7 @@ export function normalizeLaunchMpvExtraArgs(argv: string[]): string[] {
}
if (token.startsWith('--')) {
extraArgs.push(token);
if (!token.includes('=') && i + 1 < argv.length) {
if (consumesLaunchMpvValue(token) && i + 1 < argv.length) {
const value = argv[i + 1];
if (value && !value.startsWith('-')) {
extraArgs.push(value);

View File

@@ -125,6 +125,17 @@ function createFakeMpvClient(options: {
};
}
function createDeferred<T>(): {
promise: Promise<T>;
resolve: (value: T) => void;
} {
let resolve!: (value: T) => void;
const promise = new Promise<T>((settle) => {
resolve = settle;
});
return { promise, resolve };
}
test('getPlaylistBrowserSnapshotRuntime lists sibling videos in best-effort episode order', async (t) => {
const dir = createTempVideoDir(t);
const episode2 = path.join(dir, 'Show - S01E02.mkv');
@@ -488,6 +499,73 @@ test('playPlaylistBrowserIndexRuntime ignores superseded local subtitle rearm ca
);
});
test('playPlaylistBrowserIndexRuntime aborts stale async subtitle rearm work', async (t) => {
const dir = createTempVideoDir(t);
const episode1 = path.join(dir, 'Show - S01E01.mkv');
const episode2 = path.join(dir, 'Show - S01E02.mkv');
fs.writeFileSync(episode1, '');
fs.writeFileSync(episode2, '');
const firstTrackList = createDeferred<unknown>();
const secondTrackList = createDeferred<unknown>();
let trackListRequestCount = 0;
const mpvClient = createFakeMpvClient({
currentVideoPath: episode1,
playlist: [
{ filename: episode1, current: true, title: 'Episode 1' },
{ filename: episode2, title: 'Episode 2' },
],
});
const requestProperty = mpvClient.requestProperty.bind(mpvClient);
mpvClient.requestProperty = async (name: string): Promise<unknown> => {
if (name === 'track-list') {
trackListRequestCount += 1;
return trackListRequestCount === 1 ? firstTrackList.promise : secondTrackList.promise;
}
return requestProperty(name);
};
const scheduled: Array<() => void> = [];
const deps = {
getMpvClient: () => mpvClient,
schedule: (callback: () => void) => {
scheduled.push(callback);
},
};
const firstPlay = await playPlaylistBrowserIndexRuntime(deps, 1);
assert.equal(firstPlay.ok, true);
scheduled[0]?.();
const secondPlay = await playPlaylistBrowserIndexRuntime(deps, 1);
assert.equal(secondPlay.ok, true);
scheduled[1]?.();
secondTrackList.resolve([
{ type: 'sub', id: 21, lang: 'ja', title: 'Japanese', external: false, selected: true },
{ type: 'sub', id: 22, lang: 'en', title: 'English', external: false },
]);
await new Promise((resolve) => setTimeout(resolve, 0));
firstTrackList.resolve([
{ type: 'sub', id: 11, lang: 'ja', title: 'Japanese', external: false, selected: true },
{ type: 'sub', id: 12, lang: 'en', title: 'English', external: false },
]);
await new Promise((resolve) => setTimeout(resolve, 0));
const subtitleCommands = mpvClient
.getCommands()
.filter(
(command) =>
command[0] === 'set_property' && (command[1] === 'sid' || command[1] === 'secondary-sid'),
);
assert.deepEqual(subtitleCommands, [
['set_property', 'sid', 21],
['set_property', 'secondary-sid', 22],
]);
});
test('playlist-browser playback reapplies configured preferred subtitle tracks when track metadata is available', async (t) => {
const dir = createTempVideoDir(t);
const episode1 = path.join(dir, 'Show - S01E01.mkv');

View File

@@ -235,8 +235,18 @@ async function buildMutationResult(
async function rearmLocalSubtitleSelection(
client: MpvPlaylistBrowserClientLike,
deps: PlaylistBrowserRuntimeDeps,
expectedPath: string,
token: number,
): Promise<void> {
const trackList = await readProperty(client, 'track-list');
if (pendingLocalSubtitleSelectionRearms.get(client) !== token) {
return;
}
const currentPath = trimToNull(client.currentVideoPath);
if (currentPath && path.resolve(currentPath) !== expectedPath) {
return;
}
pendingLocalSubtitleSelectionRearms.delete(client);
const selection = resolveManagedLocalSubtitleSelection({
trackList: Array.isArray(trackList) ? trackList : null,
primaryLanguages: deps.getPrimarySubtitleLanguages?.() ?? [],
@@ -267,12 +277,7 @@ function scheduleLocalSubtitleSelectionRearm(
pendingLocalSubtitleSelectionRearms.set(client, nextToken);
(deps.schedule ?? setTimeout)(() => {
if (pendingLocalSubtitleSelectionRearms.get(client) !== nextToken) return;
pendingLocalSubtitleSelectionRearms.delete(client);
const currentPath = trimToNull(client.currentVideoPath);
if (currentPath && path.resolve(currentPath) !== expectedPath) {
return;
}
void rearmLocalSubtitleSelection(client, deps);
void rearmLocalSubtitleSelection(client, deps, expectedPath, nextToken);
}, 400);
}