From 7a869ad2918f4cae55810a40be2e2bfa5e8a267e Mon Sep 17 00:00:00 2001 From: sudacode Date: Sat, 21 Feb 2026 15:52:19 -0800 Subject: [PATCH] fix(config): enforce strict startup config parsing --- ...ng-strict-with-clear-user-facing-errors.md | 37 ++++++++++++---- ...ct-startup-config-20260221T231804Z-3ngd.md | 44 +++++++++++++++++++ src/config/config.test.ts | 20 ++++++++- src/config/service.ts | 23 ++++++++-- src/main/config-validation.test.ts | 10 +++++ src/main/config-validation.ts | 11 +++++ src/main/runtime/startup-config.test.ts | 12 +++-- src/main/runtime/startup-config.ts | 3 +- 8 files changed, 140 insertions(+), 20 deletions(-) create mode 100644 docs/subagents/agents/codex-task72-strict-startup-config-20260221T231804Z-3ngd.md diff --git a/backlog/tasks/task-72 - Make-startup-config-loading-strict-with-clear-user-facing-errors.md b/backlog/tasks/task-72 - Make-startup-config-loading-strict-with-clear-user-facing-errors.md index 09dfec2..b805eb7 100644 --- a/backlog/tasks/task-72 - Make-startup-config-loading-strict-with-clear-user-facing-errors.md +++ b/backlog/tasks/task-72 - Make-startup-config-loading-strict-with-clear-user-facing-errors.md @@ -1,10 +1,10 @@ --- id: TASK-72 title: Make startup config loading strict with clear user-facing errors -status: To Do +status: Done assignee: [] created_date: '2026-02-18 11:35' -updated_date: '2026-02-18 11:35' +updated_date: '2026-02-21 23:26' labels: - config - ux @@ -31,14 +31,35 @@ Startup config loading currently falls back to default config when strict parse ## Acceptance Criteria -- [ ] #1 Startup and hot-reload use consistent strictness semantics -- [ ] #2 Malformed config surfaces explicit actionable error -- [ ] #3 Tests cover malformed startup config path +- [x] #1 Startup and hot-reload use consistent strictness semantics +- [x] #2 Malformed config surfaces explicit actionable error +- [x] #3 Tests cover malformed startup config path +## Implementation Notes + + +2026-02-21: implementing plan Task 1 only in this run (strict constructor startup load + malformed startup config test coverage), no commit. + +2026-02-21 Task 1 complete: `ConfigService` constructor now strict-loads startup config and throws `ConfigStartupParseError` (path + parse reason + restart guidance) on malformed parse; added malformed-startup constructor test in `src/config/config.test.ts`; verified with `bun run build && node --test dist/config/config.test.js` (pass). + +2026-02-21 Task 2-4 complete: added shared `buildConfigParseErrorDetails` formatter and reused it in startup/hot-reload parse-failure handling (`src/main/config-validation.ts`, `src/main/runtime/startup-config.ts`). + +2026-02-21 startup wiring updated in `src/main.ts`: `ConfigService` construction now catches `ConfigStartupParseError` and fails startup via `failStartupFromConfig` with user-facing dialog + actionable details, keeping non-parse failures rethrown. + +2026-02-21 docs updated in `docs/configuration.md` to clarify malformed JSON/JSONC is startup-blocking while value-level validation remains warn-and-fallback. + +Verification: `bun test src/config/config.test.ts src/main/config-validation.test.ts src/main/runtime/startup-config.test.ts` => pass (46/46). Full `bun run build` currently blocked by unrelated pre-existing TS errors in in-flight TASK-75/TASK-77 files (`src/main/runtime/mpv-osd-log.test.ts`, `src/main/runtime/mpv-osd-runtime-handlers.test.ts`, `src/core/services/tokenizer/parser-selection-stage.test.ts`). + + +## Final Summary + + +Startup config loading is now strict and aligned with hot-reload semantics. `ConfigService` no longer silently falls back on malformed startup config; it throws a typed `ConfigStartupParseError`, and `main.ts` now converts that into the same actionable user-facing startup failure path used elsewhere (dialog/error details include config path, parse reason, and restart guidance). Added malformed-startup constructor regression coverage plus shared parse-error formatter tests, and updated configuration docs to distinguish fatal syntax errors from warn-and-fallback value validation. + + ## Definition of Done -- [ ] #1 Config and startup tests pass -- [ ] #2 Docs updated for new startup error behavior +- [x] #1 Config and startup tests pass +- [x] #2 Docs updated for new startup error behavior - diff --git a/docs/subagents/agents/codex-task72-strict-startup-config-20260221T231804Z-3ngd.md b/docs/subagents/agents/codex-task72-strict-startup-config-20260221T231804Z-3ngd.md new file mode 100644 index 0000000..2138735 --- /dev/null +++ b/docs/subagents/agents/codex-task72-strict-startup-config-20260221T231804Z-3ngd.md @@ -0,0 +1,44 @@ +# Agent: `codex-task72-strict-startup-config-20260221T231804Z-3ngd` + +- alias: `codex-task72-strict-startup-config` +- mission: `Execute TASK-72 strict startup config loading with actionable user-facing errors` +- status: `done` +- branch: `main` +- started_at: `2026-02-21T23:18:04Z` +- heartbeat_minutes: `5` + +## Current Work (newest first) + +- [2026-02-21T23:26:29Z] handoff: Completed TASK-72 end-to-end (no commit): strict startup constructor loading, shared parse-error formatter wiring, startup fail-fast guard in `main.ts`, docs update, focused tests pass, backlog task finalized to Done. +- [2026-02-21T23:26:29Z] test: `bun test src/config/config.test.ts src/main/config-validation.test.ts src/main/runtime/startup-config.test.ts` passed (46/46); full `bun run build` blocked by unrelated pre-existing TASK-75/TASK-77 TypeScript errors outside TASK-72 scope. +- [2026-02-21T23:24:20Z] progress: executed plan Tasks 1-2 via parallel subagents (`opencode-task72-strict-startup-config-*`, `opencode-task72-parse-details-*`), then integrated Task 3 (`src/main.ts`) and Task 4 (`docs/configuration.md`) in-session. +- [2026-02-21T23:19:40Z] progress: wrote execution plan at `docs/plans/2026-02-21-task-72-strict-startup-config-loading.md` via writing-plans skill. +- [2026-02-21T23:18:04Z] intent: Load TASK-72 context from Backlog MCP, produce execution plan with writing-plans skill, then implement/test/docs updates without commit. + +## Files Touched + +- `docs/subagents/INDEX.md` +- `docs/subagents/collaboration.md` +- `docs/subagents/agents/codex-task72-strict-startup-config-20260221T231804Z-3ngd.md` +- `docs/plans/2026-02-21-task-72-strict-startup-config-loading.md` +- `src/config/service.ts` +- `src/config/config.test.ts` +- `src/main/config-validation.ts` +- `src/main/config-validation.test.ts` +- `src/main/runtime/startup-config.ts` +- `src/main/runtime/startup-config.test.ts` +- `src/main.ts` +- `docs/configuration.md` +- `backlog/tasks/task-72 - Make-startup-config-loading-strict-with-clear-user-facing-errors.md` + +## Assumptions + +- Backlog MCP is available; `TASK-72` task file is source of truth for AC/DoD. + +## Open Questions / Blockers + +- None. + +## Next Step + +- Await user review or follow-up tasks. diff --git a/src/config/config.test.ts b/src/config/config.test.ts index ba804a8..d87cbe2 100644 --- a/src/config/config.test.ts +++ b/src/config/config.test.ts @@ -3,7 +3,7 @@ import assert from 'node:assert/strict'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { ConfigService } from './service'; +import { ConfigService, ConfigStartupParseError } from './service'; import { DEFAULT_CONFIG, RUNTIME_OPTION_REGISTRY } from './definitions'; import { generateConfigTemplate } from './template'; @@ -39,6 +39,24 @@ test('loads defaults when config is missing', () => { assert.equal(config.immersionTracking.retention.vacuumIntervalDays, 7); }); +test('throws actionable startup parse error for malformed config at construction time', () => { + const dir = makeTempDir(); + const configPath = path.join(dir, 'config.jsonc'); + fs.writeFileSync(configPath, '{"websocket":', 'utf-8'); + + assert.throws( + () => new ConfigService(dir), + (error: unknown) => { + assert.ok(error instanceof ConfigStartupParseError); + assert.equal(error.path, configPath); + assert.ok(error.parseError.length > 0); + assert.ok(error.message.includes(configPath)); + assert.ok(error.message.includes(error.parseError)); + return true; + }, + ); +}); + test('parses subtitleStyle.preserveLineBreaks and warns on invalid values', () => { const validDir = makeTempDir(); fs.writeFileSync( diff --git a/src/config/service.ts b/src/config/service.ts index 56d0c05..339c581 100644 --- a/src/config/service.ts +++ b/src/config/service.ts @@ -18,12 +18,26 @@ export type ReloadConfigStrictResult = path: string; }; +export class ConfigStartupParseError extends Error { + readonly path: string; + readonly parseError: string; + + constructor(configPath: string, parseError: string) { + super( + `Failed to parse startup config at ${configPath}: ${parseError}. Fix the config file and restart SubMiner.`, + ); + this.name = 'ConfigStartupParseError'; + this.path = configPath; + this.parseError = parseError; + } +} + export class ConfigService { private readonly configPaths: ConfigPaths; private rawConfig: RawConfig = {}; private resolvedConfig: ResolvedConfig = deepCloneConfig(DEFAULT_CONFIG); private warnings: ConfigValidationWarning[] = []; - private configPathInUse: string; + private configPathInUse!: string; constructor(configDir: string) { this.configPaths = { @@ -31,8 +45,11 @@ export class ConfigService { configFileJsonc: path.join(configDir, 'config.jsonc'), configFileJson: path.join(configDir, 'config.json'), }; - this.configPathInUse = this.configPaths.configFileJsonc; - this.reloadConfig(); + const loadResult = loadRawConfigStrict(this.configPaths); + if (!loadResult.ok) { + throw new ConfigStartupParseError(loadResult.path, loadResult.error); + } + this.applyResolvedConfig(loadResult.config, loadResult.path); } getConfigPath(): string { diff --git a/src/main/config-validation.test.ts b/src/main/config-validation.test.ts index 139fb45..f9aa870 100644 --- a/src/main/config-validation.test.ts +++ b/src/main/config-validation.test.ts @@ -1,6 +1,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { + buildConfigParseErrorDetails, buildConfigWarningNotificationBody, buildConfigWarningSummary, failStartupFromConfig, @@ -52,6 +53,15 @@ test('buildConfigWarningNotificationBody includes concise warning details', () = ); }); +test('buildConfigParseErrorDetails includes path error and restart guidance', () => { + const details = buildConfigParseErrorDetails('/tmp/config.jsonc', 'unexpected token at line 1'); + + assert.match(details, /Failed to parse config file at:/); + assert.match(details, /\/tmp\/config\.jsonc/); + assert.match(details, /Error: unexpected token at line 1/); + assert.match(details, /Fix the config file and restart SubMiner\./); +}); + test('failStartupFromConfig invokes handlers and throws', () => { const calls: string[] = []; const previousExitCode = process.exitCode; diff --git a/src/main/config-validation.ts b/src/main/config-validation.ts index 0e7914e..a9c326f 100644 --- a/src/main/config-validation.ts +++ b/src/main/config-validation.ts @@ -61,6 +61,17 @@ export function buildConfigWarningNotificationBody( ].join('\n'); } +export function buildConfigParseErrorDetails(configPath: string, parseError: string): string { + return [ + 'Failed to parse config file at:', + configPath, + '', + `Error: ${parseError}`, + '', + 'Fix the config file and restart SubMiner.', + ].join('\n'); +} + export function failStartupFromConfig( title: string, details: string, diff --git a/src/main/runtime/startup-config.test.ts b/src/main/runtime/startup-config.test.ts index 2645a77..742ece7 100644 --- a/src/main/runtime/startup-config.test.ts +++ b/src/main/runtime/startup-config.test.ts @@ -1,9 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { - createCriticalConfigErrorHandler, - createReloadConfigHandler, -} from './startup-config'; +import { createCriticalConfigErrorHandler, createReloadConfigHandler } from './startup-config'; test('createReloadConfigHandler runs success flow with warnings', async () => { const calls: string[] = []; @@ -42,9 +39,7 @@ test('createReloadConfigHandler runs success flow with warnings', async () => { assert.ok(calls.some((entry) => entry.startsWith('info:Using config file: /tmp/config.jsonc'))); assert.ok(calls.some((entry) => entry.startsWith('warn:[config] Validation found 1 issue(s)'))); assert.ok( - calls.some((entry) => - entry.includes('notify:SubMiner:1 config validation issue(s) detected.'), - ), + calls.some((entry) => entry.includes('notify:SubMiner:1 config validation issue(s) detected.')), ); assert.ok(calls.some((entry) => entry.includes('1. ankiConnect.pollingRate: must be >= 50'))); assert.ok(calls.includes('hotReload:start')); @@ -79,6 +74,9 @@ test('createReloadConfigHandler fails startup for parse errors', () => { assert.throws(() => reloadConfig(), /Failed to parse config file at:/); assert.equal(process.exitCode, 1); assert.ok(calls.some((entry) => entry.startsWith('error:Failed to parse config file at:'))); + assert.ok(calls.some((entry) => entry.includes('/tmp/config.jsonc'))); + assert.ok(calls.some((entry) => entry.includes('Error: unexpected token'))); + assert.ok(calls.some((entry) => entry.includes('Fix the config file and restart SubMiner.'))); assert.ok( calls.some((entry) => entry.startsWith('dialog:SubMiner config parse error:Failed to parse config file at:'), diff --git a/src/main/runtime/startup-config.ts b/src/main/runtime/startup-config.ts index 2258cd0..74f26e6 100644 --- a/src/main/runtime/startup-config.ts +++ b/src/main/runtime/startup-config.ts @@ -1,5 +1,6 @@ import type { ConfigValidationWarning } from '../../types'; import { + buildConfigParseErrorDetails, buildConfigWarningNotificationBody, buildConfigWarningSummary, failStartupFromConfig, @@ -48,7 +49,7 @@ export function createReloadConfigHandler(deps: ReloadConfigRuntimeDeps): () => if (!result.ok) { failStartupFromConfig( 'SubMiner config parse error', - `Failed to parse config file at:\n${result.path}\n\nError: ${result.error}\n\nFix the config file and restart SubMiner.`, + buildConfigParseErrorDetails(result.path, result.error), deps.failHandlers, ); }