diff --git a/backlog/tasks/task-70 - Unify-config-path-resolution-across-main-and-launcher.md b/backlog/tasks/task-70 - Unify-config-path-resolution-across-main-and-launcher.md index 7976845..13163fb 100644 --- a/backlog/tasks/task-70 - Unify-config-path-resolution-across-main-and-launcher.md +++ b/backlog/tasks/task-70 - Unify-config-path-resolution-across-main-and-launcher.md @@ -1,10 +1,11 @@ --- id: TASK-70 title: Unify config path resolution across main and launcher -status: To Do -assignee: [] +status: Done +assignee: + - codex-main created_date: '2026-02-18 11:35' -updated_date: '2026-02-18 11:35' +updated_date: '2026-02-19 09:05' labels: - config - launcher @@ -32,14 +33,46 @@ Config discovery logic is duplicated and inconsistent between app main process a ## Acceptance Criteria -- [ ] #1 Single canonical path-resolution logic used by app and launcher -- [ ] #2 `XDG_CONFIG_HOME` and `SubMiner|subminer` precedence covered by tests -- [ ] #3 No behavior drift for existing config-path CLI commands +- [x] #1 Single canonical path-resolution logic used by app and launcher +- [x] #2 `XDG_CONFIG_HOME` and `SubMiner|subminer` precedence covered by tests +- [x] #3 No behavior drift for existing config-path CLI commands +## Implementation Plan + + +1. Add focused precedence tests in `src/config/path-resolution.test.ts` for XDG/home base-dir order, SubMiner/subminer fallbacks, config.jsonc/config.json preference, and fallback path behavior. +2. Create canonical helper module `src/config/path-resolution.ts` exporting shared discovery functions for config dir and config file resolution. +3. Replace duplicated path-resolution logic in `src/main.ts`, `launcher/main.ts`, and launcher config loaders in `launcher/config.ts` to use the canonical helper. +4. Verify no behavior drift with `bun run build`, config/path tests, and launcher bundle build; then update backlog acceptance/DoD checks and execution notes. + + +## Implementation Notes + + +2026-02-19T08:52:23Z: Started task execution; gathering code context for unified config-path resolution across main and launcher. + +Plan captured in docs/plans/2026-02-19-task-70-unify-config-path-resolution.md and recorded in task. User requested immediate execution after planning. + +Implemented canonical config path discovery in `src/config/path-resolution.ts` and switched `src/main.ts`, `launcher/main.ts`, and launcher config loaders in `launcher/config.ts` to use it. + +Added precedence regression coverage in `src/config/path-resolution.test.ts` and wired into `test:config:dist`. + +Verification passed: `bun run build`, `node --test dist/config/path-resolution.test.js dist/config/config.test.js`, and `bun build ./launcher/main.ts --target=bun --packages=bundle --outfile=/tmp/subminer-task70`. + + +## Final Summary + + +Unified config discovery behind a single canonical utility at `src/config/path-resolution.ts` and replaced duplicated resolvers in `src/main.ts`, `launcher/main.ts`, and launcher config loaders in `launcher/config.ts`. This keeps `XDG_CONFIG_HOME` + `~/.config` base-dir handling, `SubMiner|subminer` case variants, and `config.jsonc` > `config.json` file preference consistent across app and launcher while preserving `subminer config path|show` fallback output behavior. + +Added regression tests in `src/config/path-resolution.test.ts` for base-dir trimming/dedup, candidate precedence, lowercase fallback, directory fallback for main config dir resolution, and fallback file-path behavior; wired the new suite into `test:config:dist` in `package.json`. + +Verification run: `bun run build`, `node --test dist/config/path-resolution.test.js dist/config/config.test.js`, `bun build ./launcher/main.ts --target=bun --packages=bundle --outfile=/tmp/subminer-task70` (all pass). + + ## Definition of Done -- [ ] #1 Launcher and config tests pass -- [ ] #2 Code no longer duplicates config path candidate logic +- [x] #1 Launcher and config tests pass +- [x] #2 Code no longer duplicates config path candidate logic - diff --git a/docs/subagents/INDEX.md b/docs/subagents/INDEX.md index 6ff0044..910d8f9 100644 --- a/docs/subagents/INDEX.md +++ b/docs/subagents/INDEX.md @@ -2,6 +2,6 @@ Read first. Keep concise. -| agent_id | alias | mission | status | file | last_update_utc | -| ------------ | -------------- | -------------------------------------------- | --------- | ------------------------------------- | ---------------------- | -| `codex-main` | `planner-exec` | `Track config-gated keybinding task request` | `handoff` | `docs/subagents/agents/codex-main.md` | `2026-02-19T08:41:44Z` | +| agent_id | alias | mission | status | file | last_update_utc | +| ------------ | -------------- | ---------------------------------------------------- | --------- | ------------------------------------- | ---------------------- | +| `codex-main` | `planner-exec` | `Unify config path resolution across app + launcher` | `handoff` | `docs/subagents/agents/codex-main.md` | `2026-02-19T09:05:26Z` | diff --git a/docs/subagents/agents/codex-main.md b/docs/subagents/agents/codex-main.md index da412d4..5e564af 100644 --- a/docs/subagents/agents/codex-main.md +++ b/docs/subagents/agents/codex-main.md @@ -1,7 +1,7 @@ # Agent: codex-main - alias: planner-exec -- mission: Track config-gated keybinding task request +- mission: Unify config path resolution across app + launcher - status: handoff - branch: main - started_at: 2026-02-19T08:06:28Z @@ -9,6 +9,10 @@ ## Current Work (newest first) +- [2026-02-19T09:05:26Z] handoff: completed TASK-70 (Done); unified config path resolution into shared `src/config/path-resolution.ts`, wired app+launcher call sites, added precedence tests, verified build/tests/launcher bundle, and checked AC/DoD in Backlog. +- [2026-02-19T09:05:26Z] test: `bun run build && node --test dist/config/path-resolution.test.js dist/config/config.test.js && bun build ./launcher/main.ts --target=bun --packages=bundle --outfile=/tmp/subminer-task70` -> pass. +- [2026-02-19T08:57:36Z] progress: plan saved to `docs/plans/2026-02-19-task-70-unify-config-path-resolution.md`, mirrored to backlog TASK-70 `planSet`; moving to edit/test execution. +- [2026-02-19T08:52:23Z] intent: execute TASK-70 by reading full backlog context, writing implementation plan via writing-plans skill, then executing via executing-plans skill with parallel subagents where possible. - [2026-02-19T08:41:44Z] handoff: created backlog TASK-84 for config-gated keybindings + disabled-feature integration non-loading (Jellyfin example) with tests/docs acceptance criteria. - [2026-02-19T08:41:22Z] intent: create backlog task for config-gated keybindings so disabled features become no-ops and feature modules (example: Jellyfin) are not loaded when disabled. - [2026-02-19T08:40:53Z] handoff: completed TASK-83 simplification; removed configurable sentence/audio field overrides for Lapis sentence cards and verified. @@ -31,6 +35,14 @@ - `docs/subagents/INDEX.md` - `docs/subagents/agents/codex-main.md` +- `docs/plans/2026-02-19-task-70-unify-config-path-resolution.md` +- `src/config/path-resolution.ts` +- `src/config/path-resolution.test.ts` +- `launcher/main.ts` +- `launcher/config.ts` +- `src/main.ts` +- `package.json` +- `backlog/tasks/task-70 - Unify-config-path-resolution-across-main-and-launcher.md` - `backlog/tasks/task-84 - Gate-feature-dependent-keybindings-behind-config-flags.md` - `src/config/service.ts` - `src/config/config.test.ts` @@ -61,4 +73,4 @@ ## Next Step -- Await user prioritization / implementation request for TASK-84. +- Await user review; optional next step is commit for TASK-70 changes. diff --git a/launcher/config.ts b/launcher/config.ts index e6752df..c482b58 100644 --- a/launcher/config.ts +++ b/launcher/config.ts @@ -3,6 +3,7 @@ import path from 'node:path'; import os from 'node:os'; import { Command } from 'commander'; import { parse as parseJsonc } from 'jsonc-parser'; +import { resolveConfigFilePath } from '../src/config/path-resolution.js'; import type { LogLevel, YoutubeSubgenMode, @@ -27,12 +28,17 @@ import { inferWhisperLanguage, } from './util.js'; +function resolveLauncherMainConfigPath(): string { + return resolveConfigFilePath({ + xdgConfigHome: process.env.XDG_CONFIG_HOME, + homeDir: os.homedir(), + existsSync: fs.existsSync, + }); +} + export function loadLauncherYoutubeSubgenConfig(): LauncherYoutubeSubgenConfig { - const configDir = path.join(os.homedir(), '.config', 'SubMiner'); - const jsoncPath = path.join(configDir, 'config.jsonc'); - const jsonPath = path.join(configDir, 'config.json'); - const configPath = fs.existsSync(jsoncPath) ? jsoncPath : fs.existsSync(jsonPath) ? jsonPath : ''; - if (!configPath) return {}; + const configPath = resolveLauncherMainConfigPath(); + if (!fs.existsSync(configPath)) return {}; try { const data = fs.readFileSync(configPath, 'utf8'); @@ -118,11 +124,8 @@ export function loadLauncherYoutubeSubgenConfig(): LauncherYoutubeSubgenConfig { } export function loadLauncherJellyfinConfig(): LauncherJellyfinConfig { - const configDir = path.join(os.homedir(), '.config', 'SubMiner'); - const jsoncPath = path.join(configDir, 'config.jsonc'); - const jsonPath = path.join(configDir, 'config.json'); - const configPath = fs.existsSync(jsoncPath) ? jsoncPath : fs.existsSync(jsonPath) ? jsonPath : ''; - if (!configPath) return {}; + const configPath = resolveLauncherMainConfigPath(); + if (!fs.existsSync(configPath)) return {}; try { const data = fs.readFileSync(configPath, 'utf8'); diff --git a/launcher/main.ts b/launcher/main.ts index b9e7044..ff6e98a 100644 --- a/launcher/main.ts +++ b/launcher/main.ts @@ -1,6 +1,7 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; +import { resolveConfigFilePath } from '../src/config/path-resolution.js'; import type { Args } from './types.js'; import { log, fail } from './log.js'; import { commandExists, isYoutubeTarget, resolvePathMaybe, realpathMaybe } from './util.js'; @@ -97,18 +98,11 @@ function registerCleanup(args: Args): void { } function resolveMainConfigPath(): string { - const xdgConfigHome = process.env.XDG_CONFIG_HOME || path.join(os.homedir(), '.config'); - const baseDirs = Array.from(new Set([xdgConfigHome, path.join(os.homedir(), '.config')])); - const appNames = ['SubMiner', 'subminer']; - for (const baseDir of baseDirs) { - for (const appName of appNames) { - const jsoncPath = path.join(baseDir, appName, 'config.jsonc'); - if (fs.existsSync(jsoncPath)) return jsoncPath; - const jsonPath = path.join(baseDir, appName, 'config.json'); - if (fs.existsSync(jsonPath)) return jsonPath; - } - } - return path.join(baseDirs[0], 'SubMiner', 'config.jsonc'); + return resolveConfigFilePath({ + xdgConfigHome: process.env.XDG_CONFIG_HOME, + homeDir: os.homedir(), + existsSync: fs.existsSync, + }); } function runDoctor(args: Args, appPath: string | null, mpvSocketPath: string): never { diff --git a/package.json b/package.json index 5a45b2f..8f23f15 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "docs:preview": "VITE_EXTRA_EXTENSIONS=jsonc vitepress preview docs --host 0.0.0.0 --port 4173 --strictPort", "format": "prettier --write .", "format:check": "prettier --check .", - "test:config:dist": "node --test dist/config/config.test.js", + "test:config:dist": "node --test dist/config/config.test.js dist/config/path-resolution.test.js", "test:core:dist": "node --test dist/cli/args.test.js dist/cli/help.test.js dist/core/services/cli-command.test.js dist/core/services/ipc.test.js dist/core/services/field-grouping-overlay.test.js dist/core/services/numeric-shortcut-session.test.js dist/core/services/secondary-subtitle.test.js dist/core/services/mpv-render-metrics.test.js dist/core/services/overlay-content-measurement.test.js dist/core/services/mpv-control.test.js dist/core/services/mpv.test.js dist/core/services/runtime-options-ipc.test.js dist/core/services/runtime-config.test.js dist/core/services/config-hot-reload.test.js dist/core/services/tokenizer.test.js dist/core/services/subsync.test.js dist/core/services/overlay-bridge.test.js dist/core/services/overlay-manager.test.js dist/core/services/overlay-shortcut-handler.test.js dist/core/services/mining.test.js dist/core/services/anki-jimaku.test.js dist/core/services/jellyfin.test.js dist/core/services/jellyfin-remote.test.js dist/core/services/immersion-tracker-service.test.js dist/core/services/app-ready.test.js dist/core/services/startup-bootstrap.test.js dist/core/services/subtitle-processing-controller.test.js dist/core/services/anilist/anilist-token-store.test.js dist/core/services/anilist/anilist-update-queue.test.js dist/renderer/error-recovery.test.js dist/subsync/utils.test.js dist/main/anilist-url-guard.test.js dist/window-trackers/x11-tracker.test.js", "test:subtitle:dist": "echo \"Subtitle tests are currently not configured\"", "test": "bun run test:config && bun run test:core", diff --git a/src/config/path-resolution.test.ts b/src/config/path-resolution.test.ts new file mode 100644 index 0000000..3cec3bc --- /dev/null +++ b/src/config/path-resolution.test.ts @@ -0,0 +1,89 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import path from 'node:path'; +import { resolveConfigBaseDirs, resolveConfigDir, resolveConfigFilePath } from './path-resolution'; + +function existsSyncFrom(paths: string[]): (candidate: string) => boolean { + const normalized = new Set(paths.map((entry) => path.normalize(entry))); + return (candidate: string): boolean => normalized.has(path.normalize(candidate)); +} + +test('resolveConfigBaseDirs trims xdg value and deduplicates fallback dir', () => { + const homeDir = '/home/tester'; + const baseDirs = resolveConfigBaseDirs(' /home/tester/.config ', homeDir); + assert.deepEqual(baseDirs, [path.join(homeDir, '.config')]); +}); + +test('resolveConfigDir prefers xdg SubMiner config when present', () => { + const homeDir = '/home/tester'; + const xdgConfigHome = '/tmp/xdg-config'; + const configDir = path.join(xdgConfigHome, 'SubMiner'); + const existsSync = existsSyncFrom([path.join(configDir, 'config.jsonc')]); + + const resolved = resolveConfigDir({ + xdgConfigHome, + homeDir, + existsSync, + }); + + assert.equal(resolved, configDir); +}); + +test('resolveConfigDir falls back to lowercase subminer candidate', () => { + const homeDir = '/home/tester'; + const configDir = path.join(homeDir, '.config', 'subminer'); + const existsSync = existsSyncFrom([path.join(configDir, 'config.json')]); + + const resolved = resolveConfigDir({ + xdgConfigHome: '/tmp/missing-xdg', + homeDir, + existsSync, + }); + + assert.equal(resolved, configDir); +}); + +test('resolveConfigDir falls back to existing directory when file is missing', () => { + const homeDir = '/home/tester'; + const configDir = path.join(homeDir, '.config', 'subminer'); + const existsSync = existsSyncFrom([configDir]); + + const resolved = resolveConfigDir({ + xdgConfigHome: '/tmp/missing-xdg', + homeDir, + existsSync, + }); + + assert.equal(resolved, configDir); +}); + +test('resolveConfigFilePath prefers jsonc before json', () => { + const homeDir = '/home/tester'; + const xdgConfigHome = '/tmp/xdg-config'; + const existsSync = existsSyncFrom([ + path.join(xdgConfigHome, 'SubMiner', 'config.jsonc'), + path.join(xdgConfigHome, 'SubMiner', 'config.json'), + ]); + + const resolved = resolveConfigFilePath({ + xdgConfigHome, + homeDir, + existsSync, + }); + + assert.equal(resolved, path.join(xdgConfigHome, 'SubMiner', 'config.jsonc')); +}); + +test('resolveConfigFilePath keeps legacy fallback output path', () => { + const homeDir = '/home/tester'; + const xdgConfigHome = '/tmp/xdg-config'; + const existsSync = existsSyncFrom([]); + + const resolved = resolveConfigFilePath({ + xdgConfigHome, + homeDir, + existsSync, + }); + + assert.equal(resolved, path.join(xdgConfigHome, 'SubMiner', 'config.jsonc')); +}); diff --git a/src/config/path-resolution.ts b/src/config/path-resolution.ts new file mode 100644 index 0000000..50cc9b7 --- /dev/null +++ b/src/config/path-resolution.ts @@ -0,0 +1,76 @@ +import path from 'node:path'; + +type ExistsSync = (candidate: string) => boolean; + +type ConfigPathOptions = { + xdgConfigHome?: string; + homeDir: string; + existsSync: ExistsSync; + appNames?: readonly string[]; + defaultAppName?: string; +}; + +const DEFAULT_APP_NAMES = ['SubMiner', 'subminer'] as const; +const DEFAULT_FILE_NAMES = ['config.jsonc', 'config.json'] as const; + +export function resolveConfigBaseDirs( + xdgConfigHome: string | undefined, + homeDir: string, +): string[] { + const fallbackBaseDir = path.join(homeDir, '.config'); + const primaryBaseDir = xdgConfigHome?.trim() || fallbackBaseDir; + return Array.from(new Set([primaryBaseDir, fallbackBaseDir])); +} + +function getAppNames(options: ConfigPathOptions): readonly string[] { + return options.appNames ?? DEFAULT_APP_NAMES; +} + +function getDefaultAppName(options: ConfigPathOptions): string { + return options.defaultAppName ?? DEFAULT_APP_NAMES[0]; +} + +export function resolveConfigDir(options: ConfigPathOptions): string { + const baseDirs = resolveConfigBaseDirs(options.xdgConfigHome, options.homeDir); + const appNames = getAppNames(options); + + for (const baseDir of baseDirs) { + for (const appName of appNames) { + const dir = path.join(baseDir, appName); + for (const fileName of DEFAULT_FILE_NAMES) { + if (options.existsSync(path.join(dir, fileName))) { + return dir; + } + } + } + } + + for (const baseDir of baseDirs) { + for (const appName of appNames) { + const dir = path.join(baseDir, appName); + if (options.existsSync(dir)) { + return dir; + } + } + } + + return path.join(baseDirs[0], getDefaultAppName(options)); +} + +export function resolveConfigFilePath(options: ConfigPathOptions): string { + const baseDirs = resolveConfigBaseDirs(options.xdgConfigHome, options.homeDir); + const appNames = getAppNames(options); + + for (const baseDir of baseDirs) { + for (const appName of appNames) { + for (const fileName of DEFAULT_FILE_NAMES) { + const candidate = path.join(baseDir, appName, fileName); + if (options.existsSync(candidate)) { + return candidate; + } + } + } + } + + return path.join(baseDirs[0], getDefaultAppName(options), DEFAULT_FILE_NAMES[0]); +} diff --git a/src/main.ts b/src/main.ts index ba54c2f..2bcb68c 100644 --- a/src/main.ts +++ b/src/main.ts @@ -184,6 +184,7 @@ import { DEFAULT_KEYBINDINGS, generateConfigTemplate, } from './config'; +import { resolveConfigDir } from './config/path-resolution'; if (process.platform === 'linux') { app.commandLine.appendSwitch('enable-features', 'GlobalShortcutsPortal'); @@ -252,41 +253,11 @@ function applyJellyfinMpvDefaults(client: MpvIpcClient): void { sendMpvCommandRuntime(client, ['set_property', 'slang', JELLYFIN_LANG_PREF]); } -function resolveConfigDir(): string { - const xdgConfigHome = process.env.XDG_CONFIG_HOME?.trim(); - const baseDirs = Array.from( - new Set([ - xdgConfigHome || path.join(os.homedir(), '.config'), - path.join(os.homedir(), '.config'), - ]), - ); - const appNames = ['SubMiner', 'subminer']; - - for (const baseDir of baseDirs) { - for (const appName of appNames) { - const dir = path.join(baseDir, appName); - if ( - fs.existsSync(path.join(dir, 'config.jsonc')) || - fs.existsSync(path.join(dir, 'config.json')) - ) { - return dir; - } - } - } - - for (const baseDir of baseDirs) { - for (const appName of appNames) { - const dir = path.join(baseDir, appName); - if (fs.existsSync(dir)) { - return dir; - } - } - } - - return path.join(baseDirs[0], 'SubMiner'); -} - -const CONFIG_DIR = resolveConfigDir(); +const CONFIG_DIR = resolveConfigDir({ + xdgConfigHome: process.env.XDG_CONFIG_HOME, + homeDir: os.homedir(), + existsSync: fs.existsSync, +}); const USER_DATA_PATH = CONFIG_DIR; const DEFAULT_MPV_LOG_PATH = process.env.SUBMINER_MPV_LOG?.trim() || DEFAULT_MPV_LOG_FILE; const DEFAULT_IMMERSION_DB_PATH = path.join(USER_DATA_PATH, 'immersion.sqlite');