fix(electron-backend): make test suite and lint host-agnostic on Windows checkouts (#1176)

* fix(electron-backend): make test suite and lint host-agnostic across Windows/Linux checkouts

Windows checkouts (core.autocrlf=true) had 13 pre-existing jest failures
and 5 Windows-only lint errors in electron-backend while Linux CI was
green:

- embedded-mpv-native-source.spec: normalize CRLF after readFileSync so
  multi-line source assertions match on autocrlf checkouts
- worker-runtime-paths.spec: build expected candidate paths with
  path.join instead of hardcoded POSIX strings
- external-player-launch-context: join darwin-only paths (.app bundle
  executables, Homebrew Caskroom) with path.posix.join so simulated
  darwin platforms resolve correctly on win32 hosts (no-op on macOS)
- app.spec: build packaged-navigation fixtures with path.resolve +
  pathToFileURL; file:///tmp/... is not a valid win32 file URL
- lint target: quote the eslint glob. The unquoted ** was expanded by
  the POSIX shell on Linux (shallow match), so CI linted only a subset
  of files while Windows passed the literal pattern to ESLint and
  linted the full tree - the hosts checked different file sets
- fix the 5 errors full-tree linting surfaces: prefer-const in
  epg-worker.service and database.worker-connection, no-unsafe-finally
  in external-player-session-registry and embedded-mpv-native.service
  (rewritten as catch-swallow with identical semantics, pinned by new
  regression tests), intentional no-control-regex in the recording
  filename sanitizer; drop stale unused disable directives
- document the quoted-glob convention in CLAUDE.md

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: mirror the quoted-lint-glob convention into AGENTS.md

AGENTS.md already mirrors the neighbouring max-lines/baseline paragraph from
CLAUDE.md, and it requires coding conventions to stay in sync between the two
files. The quoted-glob rule landed only in CLAUDE.md, so agents bootstrapping
from AGENTS.md could reintroduce a host-dependent lint target.

Also corrects the wording in both copies: the shallow expansion happens on
macOS as well as Linux — /bin/sh has no globstar on either — and the target
still exits 0 with a broken glob, which is why this went unnoticed. Adds the
file-count check that catches it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5 authored and GitHub committed 2026-07-27 23:05:22 +02:00
1 parent 19b592badb
commit 0334296f15
14 files changed
+192 -112

No files matched your search

+1
View File
@@ -17,6 +17,7 @@ This file provides guidance to coding agents working in this repository.
- Every Nx project should keep `scope:*`, `domain:*`, and `type:*` tags in `project.json` so `@nx/enforce-module-boundaries` remains useful for humans and agents.
- See `docs/architecture/nx-workspace-boundaries.md` for the current Nx tag and alias policy.
- ESLint enforces `max-lines` on TypeScript files (target under 300, hard maximum 400). Files that predate the rule are baselined in `tools/eslint/max-lines-baseline.mjs`; after splitting a file, regenerate it with `node tools/eslint/generate-max-lines-baseline.mjs`. Never add new files to the baseline — the list must only shrink. A new file that genuinely cannot be split (for example a function serialized into another process) instead carries its own file-wide `/* eslint-disable max-lines -- <why> */`; the generator skips those files, so a justified exemption never lands in the baseline.
- Project `lint` targets that shell out to eslint must quote the glob, e.g. `eslint "apps/<project>/**/*.ts"`. An unquoted `**` is expanded by the POSIX shell on Linux and macOS (which has no `globstar`, so it matches only a shallow subset of files) while Windows passes the literal pattern to ESLint, which expands it recursively — the two hosts then lint different file sets. The target still reports success either way, so a broken glob hides missing coverage instead of failing. After changing such a target, compare the linted file count against `find <project> -name '*.ts' | wc -l`.
- Repository-specific skills are committed under `.codex/skills/`. Claude Code only discovers skills under `.claude/skills/`, so `release-notes` and `release-cut` are mirrored there and the two copies must be kept in sync; every other entry in `.claude/skills/` is personal and stays gitignored. If an external agent does not support skills, treat those files as concise ownership docs.
## Documentation After Changes
+9
View File
@@ -227,6 +227,15 @@ process) instead carries its own file-wide
`/* eslint-disable max-lines -- <why> */`; the generator skips those files, so
a justified exemption never lands in the baseline.
Project `lint` targets that shell out to eslint must quote the glob, e.g.
`eslint "apps/<project>/**/*.ts"`. An unquoted `**` is expanded by the POSIX
shell on Linux and macOS (which has no `globstar`, so it matches only a
shallow subset of files) while Windows passes the literal pattern to ESLint,
which expands it recursively — the two hosts then lint different file sets.
The target still reports success either way, so a broken glob hides missing
coverage instead of failing. After changing such a target, compare the linted
file count against `find <project> -name '*.ts' | wc -l`.
## Architecture
### Monorepo Structure (Nx Workspace)
+1 -1
View File
@@ -241,7 +241,7 @@
"{workspaceRoot}/tools/embedded-mpv/runtime-probe-contract.cjs",
"{workspaceRoot}/tools/embedded-mpv/runtime-probe-contract.d.cts"
],
"command": "eslint apps/electron-backend/**/*.ts \"apps/electron-backend/src/app/services/embedded-mpv-frame-copy-runtime.ts\" \"apps/electron-backend/src/app/services/embedded-mpv-frame-copy-runtime/**/*.ts\""
"command": "eslint \"apps/electron-backend/**/*.ts\" \"apps/electron-backend/src/app/services/embedded-mpv-frame-copy-runtime.ts\" \"apps/electron-backend/src/app/services/embedded-mpv-frame-copy-runtime/**/*.ts\""
},
"test": {
"executor": "@nx/jest:jest",
+11 -4
View File
@@ -50,6 +50,8 @@ import {
} from './app';
import App from './app';
import { app as electronApp, BrowserWindow, screen } from 'electron';
import * as path from 'path';
import { pathToFileURL } from 'url';
import { store } from './services/store.service';
type MockMainWindow = {
@@ -214,18 +216,23 @@ describe('Electron app security helpers', () => {
});
it('allows only the packaged renderer file in packaged navigation', () => {
// Resolve to host-native absolute paths: POSIX-style file URLs such
// as file:///tmp/... are not valid win32 file URLs (no drive letter).
const packagedIndexPath = path.resolve('/tmp/iptvnator/index.html');
const otherIndexPath = path.resolve('/tmp/other/index.html');
expect(
isTrustedRendererNavigationUrl(
'file:///tmp/iptvnator/index.html',
pathToFileURL(packagedIndexPath).href,
false,
'/tmp/iptvnator/index.html'
packagedIndexPath
)
).toBe(true);
expect(
isTrustedRendererNavigationUrl(
'file:///tmp/other/index.html',
pathToFileURL(otherIndexPath).href,
false,
'/tmp/iptvnator/index.html'
packagedIndexPath
)
).toBe(false);
expect(
@@ -422,7 +422,6 @@ export class EpgWorkerService {
}
let settled = false;
let timeoutId: ReturnType<typeof setTimeout>;
const settle = (fn: () => void) => {
if (settled) return;
settled = true;
@@ -430,7 +429,7 @@ export class EpgWorkerService {
fn();
};
timeoutId = setTimeout(() => {
const timeoutId = setTimeout(() => {
const errorMessage = `${options.timeoutLabel} timed out after ${
this.fetchTimeoutMs / 1000
}s`;
@@ -80,7 +80,9 @@ function resolveMacOSAppBundlePlayerPath(
return playerPath;
}
return path.join(
// .app bundle paths are always POSIX, even when a darwin platform is
// simulated on a win32 host in tests.
return path.posix.join(
appBundlePath,
'Contents',
'MacOS',
@@ -320,10 +322,12 @@ function getHomebrewCaskVlcPaths(readDirectory: ReadDirectory): string[] {
const caskroomPath = '/opt/homebrew/Caskroom/vlc';
try {
// Caskroom paths are always POSIX, even when a darwin platform is
// simulated on a win32 host in tests.
return readDirectory(caskroomPath)
.filter((entry) => entry.trim().length > 0)
.map((entry) =>
path.join(
path.posix.join(
caskroomPath,
entry,
'VLC.app',
@@ -47,6 +47,24 @@ describe('ExternalPlayerSessionRegistry', () => {
expect(registry.getActiveSessionId()).toBeNull();
});
it('marks the session closed even when the runtime close fails', async () => {
const close = jest.fn().mockRejectedValue(new Error('close failed'));
const session = registry.beginSession({
player: 'vlc',
title: 'Example',
streamUrl: 'https://example.com/video.m3u8',
});
registry.attachCloser(session.id, close);
const closed = await registry.closeSession(session.id);
expect(close).toHaveBeenCalled();
expect(closed?.status).toBe('closed');
expect(closed?.canClose).toBe(false);
expect(registry.getActiveSessionId()).toBeNull();
});
it('marks runtime failures as errors without clearing the active id', () => {
const session = registry.beginSession({
player: 'mpv',
@@ -130,8 +130,11 @@ export class ExternalPlayerSessionRegistry {
try {
await runtime.close?.();
} finally {
return this.markClosed(id);
} catch {
// Close failures must not keep the session in a live state; the
// registry still reports it as closed below.
}
return this.markClosed(id);
}
}
@@ -1,53 +1,40 @@
import { readFileSync } from 'fs';
import path from 'path';
// Checkouts with core.autocrlf=true materialize these sources with CRLF
// line endings; normalize so the multi-line assertions below match the
// LF-based expectations on every host.
function readSource(relativePath: string): string {
return readFileSync(path.resolve(__dirname, relativePath), 'utf8').replace(
/\r\n/g,
'\n'
);
}
describe('Embedded MPV native source recording invariants', () => {
const nativeSource = readFileSync(
path.resolve(__dirname, '../../../native/src/embedded_mpv.mm'),
'utf8'
const nativeSource = readSource('../../../native/src/embedded_mpv.mm');
const widCommonSource = readSource(
'../../../native/src/embedded_mpv_wid_common.h'
);
const widCommonSource = readFileSync(
path.resolve(
__dirname,
'../../../native/src/embedded_mpv_wid_common.h'
),
'utf8'
const win32Source = readSource(
'../../../native/src/embedded_mpv_win32.cc'
);
const win32Source = readFileSync(
path.resolve(__dirname, '../../../native/src/embedded_mpv_win32.cc'),
'utf8'
const linuxSource = readSource(
'../../../native/src/embedded_mpv_linux.cc'
);
const linuxSource = readFileSync(
path.resolve(__dirname, '../../../native/src/embedded_mpv_linux.cc'),
'utf8'
);
const buildScriptSource = readFileSync(
path.resolve(__dirname, '../../../build-embedded-mpv.js'),
'utf8'
);
const buildAndMakeWorkflowSource = readFileSync(
path.resolve(
__dirname,
'../../../../../.github/workflows/build-and-make.yaml'
),
'utf8'
const buildScriptSource = readSource('../../../build-embedded-mpv.js');
const buildAndMakeWorkflowSource = readSource(
'../../../../../.github/workflows/build-and-make.yaml'
);
const electronBuilderConfig = JSON.parse(
readFileSync(
path.resolve(__dirname, '../../../../../electron-builder.json'),
'utf8'
)
readSource('../../../../../electron-builder.json')
) as {
snap?: {
plugs?: unknown;
};
};
const stageRuntimeSource = readFileSync(
path.resolve(
__dirname,
'../../../../../tools/embedded-mpv/stage-runtime.mjs'
),
'utf8'
const stageRuntimeSource = readSource(
'../../../../../tools/embedded-mpv/stage-runtime.mjs'
);
const frameHelperRenderSource = readFileSync(
path.resolve(__dirname, '../../../native/helper/frame_helper_render.h'),
@@ -1112,4 +1112,21 @@ describe('EmbeddedMpvNativeService power blocker', () => {
})
);
});
it('still broadcasts a closed session when native dispose throws', () => {
startSession('s1', snapshot('playing'));
addon.disposeSession.mockImplementationOnce(() => {
throw new Error('native dispose failed');
});
const disposed = service.disposeSession('s1');
expect(disposed?.status).toBe('closed');
expect(mainWindowSendMock).toHaveBeenLastCalledWith(
'EMBEDDED_MPV_SESSION_UPDATE',
expect.objectContaining({ id: 's1', status: 'closed' })
);
// The session is removed from the registry despite the addon failure.
expect(service.disposeSession('s1')).toBeNull();
});
});
@@ -663,32 +663,35 @@ export class EmbeddedMpvNativeService {
lastRecording = undefined;
}
addon.disposeSession(sessionId);
} finally {
this.sessions.delete(sessionId);
this.pollFailuresLogged.delete(sessionId);
const payload: EmbeddedMpvSession = {
id: session.id,
title: session.title,
streamUrl: session.streamUrl,
status: 'closed',
positionSeconds: 0,
durationSeconds: null,
volume: 1,
audioTracks: [],
selectedAudioTrackId: null,
subtitleTracks: [],
selectedSubtitleTrackId: null,
playbackSpeed: 1,
aspectOverride: 'no',
recording: this.createClosedRecordingState(lastRecording),
startedAt: session.startedAt,
updatedAt: new Date().toISOString(),
};
this.sendSessionUpdate(payload);
this.stopPollingIfIdle();
this.updatePowerBlocker();
return payload;
} catch {
// Native dispose failures must not block registry cleanup or the
// closed-session broadcast below.
}
this.sessions.delete(sessionId);
this.pollFailuresLogged.delete(sessionId);
const payload: EmbeddedMpvSession = {
id: session.id,
title: session.title,
streamUrl: session.streamUrl,
status: 'closed',
positionSeconds: 0,
durationSeconds: null,
volume: 1,
audioTracks: [],
selectedAudioTrackId: null,
subtitleTracks: [],
selectedSubtitleTrackId: null,
playbackSpeed: 1,
aspectOverride: 'no',
recording: this.createClosedRecordingState(lastRecording),
startedAt: session.startedAt,
updatedAt: new Date().toISOString(),
};
this.sendSessionUpdate(payload);
this.stopPollingIfIdle();
this.updatePowerBlocker();
return payload;
}
shutdown(): void {
@@ -979,6 +982,9 @@ export class EmbeddedMpvNativeService {
private sanitizeRecordingFileName(title: string): string {
const normalized = title
// Control characters are invalid in file names on Windows; the
// range is matched intentionally.
// eslint-disable-next-line no-control-regex
.replace(/[<>:"/\\|?*\u0000-\u001f]/g, '_')
.replace(/\s+/g, ' ')
.trim();
@@ -15,7 +15,6 @@ import {
trace,
} from '../services/debug-trace';
let Database: typeof BetterSqlite3;
let drizzleFactory:
| (typeof import('drizzle-orm/better-sqlite3'))['drizzle']
| undefined;
@@ -37,7 +36,6 @@ function loadBetterSqlite3(): typeof BetterSqlite3 {
loggerLabel: '[DB Worker]',
searchPaths: nativeModuleSearchPaths,
fallbackRequire: () =>
// eslint-disable-next-line @typescript-eslint/no-require-imports
require('better-sqlite3') as typeof BetterSqlite3,
});
}
@@ -49,7 +47,6 @@ function getDrizzleFactory(): (typeof import('drizzle-orm/better-sqlite3'))['dri
// Require drizzle only after native lookup paths have been registered.
// Its better-sqlite3 driver resolves the native package at module load time.
// eslint-disable-next-line @typescript-eslint/no-require-imports
drizzleFactory = require('drizzle-orm/better-sqlite3').drizzle as (
typeof import('drizzle-orm/better-sqlite3')
)['drizzle'];
@@ -57,7 +54,7 @@ function getDrizzleFactory(): (typeof import('drizzle-orm/better-sqlite3'))['dri
return drizzleFactory;
}
Database = loadBetterSqlite3();
const Database = loadBetterSqlite3();
let db: AppDatabase | null = null;
let sqlite: BetterSqlite3.Database | null = null;
@@ -6,67 +6,101 @@ import {
resolveWorkerRuntimeBootstrap,
} from './worker-runtime-paths';
// The implementation joins candidate paths with the host path module, so the
// expected values are built with path.join too — hardcoded POSIX strings
// would fail on win32 checkouts.
function packagedWorkerPath(root: string, workerFilename: string): string {
return path.join(
root,
'dist',
'apps',
'electron-backend',
'workers',
workerFilename
);
}
function unpackedNodeModulesPaths(root: string): string[] {
return [
path.join(root, 'app.asar.unpacked', 'node_modules'),
path.join(
root,
'app.asar.unpacked',
'electron-backend',
'node_modules'
),
path.join(
root,
'app.asar.unpacked',
'dist',
'apps',
'electron-backend',
'node_modules'
),
];
}
describe('worker-runtime-paths', () => {
it('resolves the development worker path when the worker file exists', () => {
const developmentWorkerDir =
'/workspace/dist/apps/electron-backend/workers';
const workerPath = path.join(
developmentWorkerDir,
'database.worker.js'
);
const bootstrap = resolveWorkerRuntimeBootstrap({
isPackaged: false,
workerFilename: 'database.worker.js',
developmentWorkerDir: '/workspace/dist/apps/electron-backend/workers',
fileExists: (filePath) =>
filePath ===
'/workspace/dist/apps/electron-backend/workers/database.worker.js',
developmentWorkerDir,
fileExists: (filePath) => filePath === workerPath,
});
expect(bootstrap).toEqual({
workerPath:
'/workspace/dist/apps/electron-backend/workers/database.worker.js',
workerPathCandidates: [
'/workspace/dist/apps/electron-backend/workers/database.worker.js',
],
workerPath,
workerPathCandidates: [workerPath],
});
});
it('prefers process.resourcesPath for packaged worker resolution', () => {
const resourcesPath = '/Applications/IPTVnator.app/Contents/Resources';
const resourcesWorkerPath = packagedWorkerPath(
resourcesPath,
'epg-parser.worker.js'
);
const bootstrap = resolveWorkerRuntimeBootstrap({
isPackaged: true,
workerFilename: 'epg-parser.worker.js',
developmentWorkerDir: '/unused',
resourcesPath: '/Applications/IPTVnator.app/Contents/Resources',
resourcesPath,
appPath:
'/Applications/IPTVnator.app/Contents/Resources/app.asar',
fileExists: (filePath) =>
filePath ===
'/Applications/IPTVnator.app/Contents/Resources/dist/apps/electron-backend/workers/epg-parser.worker.js',
fileExists: (filePath) => filePath === resourcesWorkerPath,
});
expect(bootstrap.workerPath).toBe(
'/Applications/IPTVnator.app/Contents/Resources/dist/apps/electron-backend/workers/epg-parser.worker.js'
expect(bootstrap.workerPath).toBe(resourcesWorkerPath);
expect(bootstrap.nativeModuleSearchPaths).toEqual(
unpackedNodeModulesPaths(resourcesPath)
);
expect(bootstrap.nativeModuleSearchPaths).toEqual([
'/Applications/IPTVnator.app/Contents/Resources/app.asar.unpacked/node_modules',
'/Applications/IPTVnator.app/Contents/Resources/app.asar.unpacked/electron-backend/node_modules',
'/Applications/IPTVnator.app/Contents/Resources/app.asar.unpacked/dist/apps/electron-backend/node_modules',
]);
});
it('falls back to appPath dirname when the resourcesPath candidate is missing', () => {
const appWorkerPath = packagedWorkerPath(
'/opt/IPTVnator/resources',
'database.worker.js'
);
const bootstrap = resolveWorkerRuntimeBootstrap({
isPackaged: true,
workerFilename: 'database.worker.js',
developmentWorkerDir: '/unused',
resourcesPath: '/tmp/runtime-resources',
appPath: '/opt/IPTVnator/resources/app.asar',
fileExists: (filePath) =>
filePath ===
'/opt/IPTVnator/resources/dist/apps/electron-backend/workers/database.worker.js',
fileExists: (filePath) => filePath === appWorkerPath,
});
expect(bootstrap.workerPath).toBe(
'/opt/IPTVnator/resources/dist/apps/electron-backend/workers/database.worker.js'
);
expect(bootstrap.workerPath).toBe(appWorkerPath);
expect(bootstrap.workerPathCandidates).toEqual([
'/tmp/runtime-resources/dist/apps/electron-backend/workers/database.worker.js',
'/opt/IPTVnator/resources/dist/apps/electron-backend/workers/database.worker.js',
packagedWorkerPath('/tmp/runtime-resources', 'database.worker.js'),
appWorkerPath,
]);
});
@@ -84,8 +118,11 @@ describe('worker-runtime-paths', () => {
[
'Unable to resolve worker "database.worker.js".',
'Tried:',
'- /resources/dist/apps/electron-backend/workers/database.worker.js',
'- /opt/IPTVnator/resources/dist/apps/electron-backend/workers/database.worker.js',
`- ${packagedWorkerPath('/resources', 'database.worker.js')}`,
`- ${packagedWorkerPath(
'/opt/IPTVnator/resources',
'database.worker.js'
)}`,
].join('\n')
);
});
@@ -96,11 +133,7 @@ describe('worker-runtime-paths', () => {
resourcesPath: '/resources',
appPath: '/resources/app.asar',
})
).toEqual([
'/resources/app.asar.unpacked/node_modules',
'/resources/app.asar.unpacked/electron-backend/node_modules',
'/resources/app.asar.unpacked/dist/apps/electron-backend/node_modules',
]);
).toEqual(unpackedNodeModulesPaths('/resources'));
});
it('loads native modules from the first working candidate path', () => {
@@ -62,7 +62,6 @@ export function registerNativeModuleSearchPaths(
}
): string[] {
const env = options?.env ?? process.env;
// eslint-disable-next-line @typescript-eslint/no-require-imports
const moduleApi =
options?.moduleApi ??
(require('module') as NodeModuleApi);