mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-07-19 11:22:41 +00:00
Merge pull request #117 from vo90/agent/library-path-live-refresh
fix(library): apply saved path without restart
This commit is contained in:
commit
9e5ccdbd90
139
src/main/library-path-config.ts
Normal file
139
src/main/library-path-config.ts
Normal file
@ -0,0 +1,139 @@
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
|
||||
export type LibraryPathPreparationStatus =
|
||||
| 'explicit-override'
|
||||
| 'configured'
|
||||
| 'bootstrapped'
|
||||
| 'invalid-config'
|
||||
| 'write-failed';
|
||||
|
||||
export interface LibraryPathPreparation {
|
||||
status: LibraryPathPreparationStatus;
|
||||
environmentDlcDir?: string;
|
||||
error?: string;
|
||||
}
|
||||
|
||||
function isPlainObject(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === 'object' && value !== null && !Array.isArray(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize a configured library path and accept it only when it already
|
||||
* names a directory.
|
||||
*/
|
||||
export function normalizeExistingLibraryDirectory(rawPath?: string): string | undefined {
|
||||
const candidate = (rawPath || '').trim();
|
||||
if (!candidate) return undefined;
|
||||
|
||||
try {
|
||||
return fs.statSync(candidate).isDirectory() ? candidate : undefined;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply the prepared library-path contract to the Python child environment.
|
||||
* Starting from a copy of process.env means an invalid or stale parent value
|
||||
* must be removed explicitly when config.json owns the path.
|
||||
*/
|
||||
export function applyLibraryPathToPythonEnvironment(
|
||||
environment: Record<string, string>,
|
||||
preparation: LibraryPathPreparation,
|
||||
): void {
|
||||
if (preparation.environmentDlcDir) {
|
||||
environment.DLC_DIR = preparation.environmentDlcDir;
|
||||
} else {
|
||||
delete environment.DLC_DIR;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prepare the library path contract for the Python backend.
|
||||
*
|
||||
* An explicit DLC_DIR remains an administrator-owned environment override.
|
||||
* Normal desktop launches instead keep the selected path in config.json so
|
||||
* the backend can re-read a Settings change on the next manual scan without
|
||||
* requiring a process restart.
|
||||
*
|
||||
* Failure statuses (invalid-config, write-failed) still export the resolved
|
||||
* fallback as DLC_DIR: the backend has NO built-in default (_get_dlc_dir
|
||||
* returns None), so removing the env var when config.json cannot own the
|
||||
* path would strand the user with an empty library and only a console warn.
|
||||
* Config-owned dynamic behaviour applies exactly when config.json is usable.
|
||||
*/
|
||||
export function prepareLibraryPathForPython(
|
||||
configDir: string,
|
||||
resolvedDlcDir: string,
|
||||
explicitDlcDir?: string,
|
||||
): LibraryPathPreparation {
|
||||
const override = normalizeExistingLibraryDirectory(explicitDlcDir);
|
||||
if (override) {
|
||||
return {
|
||||
status: 'explicit-override',
|
||||
environmentDlcDir: override,
|
||||
};
|
||||
}
|
||||
|
||||
const configFile = path.join(configDir, 'config.json');
|
||||
let config: Record<string, unknown> = {};
|
||||
|
||||
if (fs.existsSync(configFile)) {
|
||||
try {
|
||||
const parsed: unknown = JSON.parse(fs.readFileSync(configFile, 'utf8'));
|
||||
if (!isPlainObject(parsed)) {
|
||||
return {
|
||||
status: 'invalid-config',
|
||||
environmentDlcDir: resolvedDlcDir,
|
||||
error: 'config.json is not a JSON object',
|
||||
};
|
||||
}
|
||||
config = parsed;
|
||||
} catch (err) {
|
||||
return {
|
||||
status: 'invalid-config',
|
||||
environmentDlcDir: resolvedDlcDir,
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
};
|
||||
}
|
||||
|
||||
const configured = config.dlc_dir;
|
||||
if (typeof configured === 'string' && configured.trim()) {
|
||||
if (normalizeExistingLibraryDirectory(configured)) {
|
||||
return { status: 'configured' };
|
||||
}
|
||||
return {
|
||||
status: 'invalid-config',
|
||||
environmentDlcDir: resolvedDlcDir,
|
||||
error: 'config.json dlc_dir is not an existing directory',
|
||||
};
|
||||
}
|
||||
if (configured !== undefined && configured !== null && configured !== '') {
|
||||
return {
|
||||
status: 'invalid-config',
|
||||
environmentDlcDir: resolvedDlcDir,
|
||||
error: 'config.json dlc_dir is not a string',
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
config.dlc_dir = resolvedDlcDir;
|
||||
const tmpFile = configFile + '.tmp';
|
||||
try {
|
||||
fs.mkdirSync(configDir, { recursive: true });
|
||||
fs.writeFileSync(tmpFile, JSON.stringify(config, null, 2), 'utf8');
|
||||
fs.renameSync(tmpFile, configFile);
|
||||
} catch (err) {
|
||||
try {
|
||||
if (fs.existsSync(tmpFile)) fs.rmSync(tmpFile, { force: true });
|
||||
} catch { /* best-effort temporary-file cleanup */ }
|
||||
return {
|
||||
status: 'write-failed',
|
||||
environmentDlcDir: resolvedDlcDir,
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
};
|
||||
}
|
||||
|
||||
return { status: 'bootstrapped' };
|
||||
}
|
||||
@ -11,6 +11,11 @@ import * as net from 'net';
|
||||
import * as os from 'os';
|
||||
import { getActiveSoundfontPath, getDesktopConfig } from './soundfont-manager';
|
||||
import { isDebugEnabled } from './debug-log';
|
||||
import {
|
||||
applyLibraryPathToPythonEnvironment,
|
||||
normalizeExistingLibraryDirectory,
|
||||
prepareLibraryPathForPython,
|
||||
} from './library-path-config';
|
||||
|
||||
let pythonProcess: ChildProcess | null = null;
|
||||
// A backend that is being *gracefully* stopped (SIGTERM sent, async SIGKILL
|
||||
@ -423,8 +428,8 @@ function getPluginsDir(): string {
|
||||
return pluginsDir;
|
||||
}
|
||||
|
||||
function getDLCDir(): string {
|
||||
if (process.env.DLC_DIR && fs.existsSync(process.env.DLC_DIR)) return process.env.DLC_DIR;
|
||||
function getDLCDir(explicitDlcDir = normalizeExistingLibraryDirectory(process.env.DLC_DIR)): string {
|
||||
if (explicitDlcDir) return explicitDlcDir;
|
||||
|
||||
// Read from shared config
|
||||
const configFile = path.join(getConfigDir(), 'config.json');
|
||||
@ -496,7 +501,8 @@ export async function startPython(): Promise<void> {
|
||||
}
|
||||
serverPort = await findPort(PREFERRED_PORT);
|
||||
const configDir = getConfigDir();
|
||||
const dlcDir = getDLCDir();
|
||||
const explicitDlcDir = normalizeExistingLibraryDirectory(process.env.DLC_DIR);
|
||||
const dlcDir = getDLCDir(explicitDlcDir);
|
||||
// Ensure the resolved library folder exists before the server starts. The
|
||||
// Python side only seeds starter content (and scans) when DLC_DIR.is_dir()
|
||||
// is true, and it can't bootstrap the folder itself (the seed's mkdir runs
|
||||
@ -508,6 +514,15 @@ export async function startPython(): Promise<void> {
|
||||
} catch (err) {
|
||||
console.warn(`[python] could not create DLC dir ${dlcDir}:`, err);
|
||||
}
|
||||
// Preserve a caller-supplied DLC_DIR as an explicit administrator override.
|
||||
// Normal desktop launches bootstrap the initial/default path into config.json
|
||||
// instead. The backend re-reads that file for every scan, so a path saved in
|
||||
// Settings takes effect immediately rather than being shadowed by the
|
||||
// startup path until the Python process restarts.
|
||||
const libraryPath = prepareLibraryPathForPython(configDir, dlcDir, explicitDlcDir);
|
||||
if (libraryPath.error) {
|
||||
console.warn(`[python] could not prepare dynamic library path (${libraryPath.status}): ${libraryPath.error}`);
|
||||
}
|
||||
const pluginsDir = getPluginsDir();
|
||||
const slopsmithPlugins = path.join(slopsmithDir, 'plugins');
|
||||
|
||||
@ -558,7 +573,6 @@ export async function startPython(): Promise<void> {
|
||||
...process.env as Record<string, string>,
|
||||
PYTHONPATH: pythonPathEnv,
|
||||
CONFIG_DIR: configDir,
|
||||
DLC_DIR: dlcDir,
|
||||
SLOPSMITH_PLUGINS_DIR: pluginsDir,
|
||||
HOME: homeDir,
|
||||
XDG_CACHE_HOME: cacheBase,
|
||||
@ -572,6 +586,9 @@ export async function startPython(): Promise<void> {
|
||||
: path.join(__dirname, '..', '..', 'resources', 'bin') + path.delimiter
|
||||
) + (process.env.PATH || ''),
|
||||
};
|
||||
// `...process.env` may carry an empty/invalid value. Do not let it shadow
|
||||
// config.json in the normal dynamic-settings path.
|
||||
applyLibraryPathToPythonEnvironment(pythonEnv, libraryPath);
|
||||
|
||||
// Debug mode: raise the Slopsmith server's log level and tee its
|
||||
// structured logs to a file. lib/logging_setup.py reads LOG_LEVEL and
|
||||
|
||||
216
tests/library-path-config.test.js
Normal file
216
tests/library-path-config.test.js
Normal file
@ -0,0 +1,216 @@
|
||||
'use strict';
|
||||
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const path = require('node:path');
|
||||
const { loadTs } = require('./_load-ts');
|
||||
|
||||
const {
|
||||
applyLibraryPathToPythonEnvironment,
|
||||
normalizeExistingLibraryDirectory,
|
||||
prepareLibraryPathForPython,
|
||||
} = loadTs('src/main/library-path-config.ts');
|
||||
|
||||
function tmpConfigDir() {
|
||||
return fs.mkdtempSync(path.join(os.tmpdir(), 'feedback-library-path-'));
|
||||
}
|
||||
|
||||
test('normal desktop startup bootstraps the fallback into config instead of DLC_DIR', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const result = prepareLibraryPathForPython(configDir, 'C:\\Music\\fee[dB]ack');
|
||||
|
||||
assert.deepEqual(result, { status: 'bootstrapped' });
|
||||
assert.deepEqual(
|
||||
JSON.parse(fs.readFileSync(path.join(configDir, 'config.json'), 'utf8')),
|
||||
{ dlc_dir: 'C:\\Music\\fee[dB]ack' },
|
||||
);
|
||||
assert.equal(result.environmentDlcDir, undefined);
|
||||
});
|
||||
|
||||
test('bootstrap merges the fallback into an existing config without losing settings', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const configFile = path.join(configDir, 'config.json');
|
||||
fs.writeFileSync(configFile, JSON.stringify({ master_difficulty: 75 }));
|
||||
|
||||
const result = prepareLibraryPathForPython(configDir, 'D:\\Songs');
|
||||
|
||||
assert.equal(result.status, 'bootstrapped');
|
||||
assert.deepEqual(
|
||||
JSON.parse(fs.readFileSync(configFile, 'utf8')),
|
||||
{ master_difficulty: 75, dlc_dir: 'D:\\Songs' },
|
||||
);
|
||||
});
|
||||
|
||||
test('an existing saved library stays config-owned and can change between scans', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const configFile = path.join(configDir, 'config.json');
|
||||
const savedSongs = path.join(configDir, 'Saved Songs');
|
||||
fs.mkdirSync(savedSongs);
|
||||
fs.writeFileSync(configFile, JSON.stringify({ dlc_dir: savedSongs }));
|
||||
|
||||
const result = prepareLibraryPathForPython(configDir, 'C:\\Default Songs');
|
||||
|
||||
assert.deepEqual(result, { status: 'configured' });
|
||||
assert.deepEqual(
|
||||
JSON.parse(fs.readFileSync(configFile, 'utf8')),
|
||||
{ dlc_dir: savedSongs },
|
||||
);
|
||||
assert.equal(result.environmentDlcDir, undefined);
|
||||
});
|
||||
|
||||
test('a missing saved library is reported without overwriting the intended path', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const configFile = path.join(configDir, 'config.json');
|
||||
const missingSongs = path.join(configDir, 'Disconnected Songs');
|
||||
fs.writeFileSync(configFile, JSON.stringify({ dlc_dir: missingSongs }));
|
||||
|
||||
const result = prepareLibraryPathForPython(configDir, 'C:\\Default Songs');
|
||||
|
||||
assert.equal(result.status, 'invalid-config');
|
||||
assert.match(result.error, /not an existing directory/);
|
||||
assert.deepEqual(JSON.parse(fs.readFileSync(configFile, 'utf8')), { dlc_dir: missingSongs });
|
||||
});
|
||||
|
||||
test('a saved library file is rejected without overwriting the configured value', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const configFile = path.join(configDir, 'config.json');
|
||||
const file = path.join(configDir, 'not-a-library');
|
||||
fs.writeFileSync(file, 'x');
|
||||
fs.writeFileSync(configFile, JSON.stringify({ dlc_dir: file }));
|
||||
|
||||
const result = prepareLibraryPathForPython(configDir, 'C:\\Default Songs');
|
||||
|
||||
assert.equal(result.status, 'invalid-config');
|
||||
assert.match(result.error, /not an existing directory/);
|
||||
assert.deepEqual(JSON.parse(fs.readFileSync(configFile, 'utf8')), { dlc_dir: file });
|
||||
});
|
||||
|
||||
test('an explicit valid DLC_DIR remains an environment override', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const managedSongs = path.join(configDir, 'Managed Songs');
|
||||
fs.mkdirSync(managedSongs);
|
||||
const result = prepareLibraryPathForPython(
|
||||
configDir,
|
||||
'C:\\Default Songs',
|
||||
` ${managedSongs} `,
|
||||
);
|
||||
|
||||
assert.deepEqual(result, {
|
||||
status: 'explicit-override',
|
||||
environmentDlcDir: managedSongs,
|
||||
});
|
||||
assert.equal(fs.existsSync(path.join(configDir, 'config.json')), false);
|
||||
});
|
||||
|
||||
test('an explicit DLC_DIR rejects whitespace, files, and missing paths', () => {
|
||||
const root = tmpConfigDir();
|
||||
const file = path.join(root, 'not-a-directory');
|
||||
fs.writeFileSync(file, 'x');
|
||||
|
||||
assert.equal(normalizeExistingLibraryDirectory(' '), undefined);
|
||||
assert.equal(normalizeExistingLibraryDirectory(file), undefined);
|
||||
assert.equal(normalizeExistingLibraryDirectory(path.join(root, 'missing')), undefined);
|
||||
});
|
||||
|
||||
test('an explicit DLC_DIR is trimmed before directory validation', () => {
|
||||
const root = tmpConfigDir();
|
||||
const directory = path.join(root, 'Managed Songs');
|
||||
fs.mkdirSync(directory);
|
||||
|
||||
assert.equal(normalizeExistingLibraryDirectory(` ${directory} `), directory);
|
||||
});
|
||||
|
||||
test('a corrupt config is never overwritten during bootstrap', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const configFile = path.join(configDir, 'config.json');
|
||||
fs.writeFileSync(configFile, '{broken');
|
||||
|
||||
const result = prepareLibraryPathForPython(configDir, 'C:\\Default Songs');
|
||||
|
||||
assert.equal(result.status, 'invalid-config');
|
||||
assert.match(result.error, /JSON/);
|
||||
assert.equal(fs.readFileSync(configFile, 'utf8'), '{broken');
|
||||
});
|
||||
|
||||
test('invalid-config exports the resolved fallback so the backend keeps a library', () => {
|
||||
// The backend has no built-in default (_get_dlc_dir returns None): with
|
||||
// the env var deleted AND config.json unusable, scans and starter
|
||||
// seeding would silently skip. Failure statuses must keep the pre-#117
|
||||
// env fallback.
|
||||
const configDir = tmpConfigDir();
|
||||
const configFile = path.join(configDir, 'config.json');
|
||||
fs.writeFileSync(configFile, JSON.stringify({ dlc_dir: path.join(configDir, 'gone') }));
|
||||
|
||||
const result = prepareLibraryPathForPython(configDir, 'C:\\Default Songs');
|
||||
|
||||
assert.equal(result.status, 'invalid-config');
|
||||
assert.equal(result.environmentDlcDir, 'C:\\Default Songs');
|
||||
|
||||
const environment = {};
|
||||
applyLibraryPathToPythonEnvironment(environment, result);
|
||||
assert.deepEqual(environment, { DLC_DIR: 'C:\\Default Songs' });
|
||||
});
|
||||
|
||||
test('corrupt config exports the resolved fallback without being overwritten', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const configFile = path.join(configDir, 'config.json');
|
||||
fs.writeFileSync(configFile, '{broken');
|
||||
|
||||
const result = prepareLibraryPathForPython(configDir, 'C:\\Default Songs');
|
||||
|
||||
assert.equal(result.status, 'invalid-config');
|
||||
assert.equal(result.environmentDlcDir, 'C:\\Default Songs');
|
||||
assert.equal(fs.readFileSync(configFile, 'utf8'), '{broken');
|
||||
});
|
||||
|
||||
test('write-failed exports the resolved fallback so the backend keeps a library', () => {
|
||||
// Force the config write to fail by making config.json's parent a file.
|
||||
const root = tmpConfigDir();
|
||||
const configDir = path.join(root, 'not-a-dir');
|
||||
fs.writeFileSync(configDir, 'x');
|
||||
|
||||
const result = prepareLibraryPathForPython(configDir, 'C:\\Default Songs');
|
||||
|
||||
assert.equal(result.status, 'write-failed');
|
||||
assert.equal(result.environmentDlcDir, 'C:\\Default Songs');
|
||||
});
|
||||
|
||||
test('Python environment omits DLC_DIR when config owns the library path', () => {
|
||||
const environment = { DLC_DIR: 'C:\\Stale Parent Value', KEEP: 'yes' };
|
||||
|
||||
applyLibraryPathToPythonEnvironment(environment, { status: 'configured' });
|
||||
|
||||
assert.deepEqual(environment, { KEEP: 'yes' });
|
||||
});
|
||||
|
||||
test('Python environment exports only a validated explicit DLC_DIR', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const managedSongs = path.join(configDir, 'Managed Songs');
|
||||
fs.mkdirSync(managedSongs);
|
||||
const preparation = prepareLibraryPathForPython(
|
||||
configDir,
|
||||
'C:\\Default Songs',
|
||||
` ${managedSongs} `,
|
||||
);
|
||||
const environment = { DLC_DIR: 'C:\\Stale Parent Value', KEEP: 'yes' };
|
||||
|
||||
applyLibraryPathToPythonEnvironment(environment, preparation);
|
||||
|
||||
assert.deepEqual(environment, { DLC_DIR: managedSongs, KEEP: 'yes' });
|
||||
});
|
||||
|
||||
test('Python environment removes an invalid explicit DLC_DIR', () => {
|
||||
const configDir = tmpConfigDir();
|
||||
const fallback = path.join(configDir, 'Fallback Songs');
|
||||
fs.mkdirSync(fallback);
|
||||
const file = path.join(configDir, 'not-a-directory');
|
||||
fs.writeFileSync(file, 'x');
|
||||
const preparation = prepareLibraryPathForPython(configDir, fallback, file);
|
||||
const environment = { DLC_DIR: file };
|
||||
|
||||
applyLibraryPathToPythonEnvironment(environment, preparation);
|
||||
|
||||
assert.deepEqual(environment, {});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user