From 12a4545124a86aa89a1af5157f3b9d6fa7c4d3c8 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Sat, 4 Apr 2026 04:11:00 -0700 Subject: [PATCH] fix(config): warn on unrecognized keys in config.json instead of silent drop (#1542) * fix(config): warn on unrecognized keys in config.json instead of silent drop (#1535) loadConfig() silently ignores any config.json keys not in its known set, leaving users confused when their settings have no effect. Add a stderr warning listing unrecognized top-level keys so the problem surfaces immediately. Co-Authored-By: Claude Opus 4.6 (1M context) * fix(config): derive known keys from VALID_CONFIG_KEYS instead of hardcoded set Address review feedback: replace hardcoded KNOWN_CONFIG_KEYS with programmatic derivation from config-set's VALID_CONFIG_KEYS (single source of truth). New config keys added to config-set are automatically recognized by loadConfig without a separate update. Add sync test verifying all VALID_CONFIG_KEYS entries pass without warning. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- get-shit-done/bin/lib/config.cjs | 1 + get-shit-done/bin/lib/core.cjs | 22 +++++++++++++ tests/core.test.cjs | 55 ++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/get-shit-done/bin/lib/config.cjs b/get-shit-done/bin/lib/config.cjs index fd1f11ae9..f36e55f19 100644 --- a/get-shit-done/bin/lib/config.cjs +++ b/get-shit-done/bin/lib/config.cjs @@ -440,6 +440,7 @@ function getCmdConfigSetModelProfileResultMessage( } module.exports = { + VALID_CONFIG_KEYS, cmdConfigEnsureSection, cmdConfigSet, cmdConfigGet, diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 4d1f6d9fb..65b440c4f 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -286,6 +286,28 @@ function loadConfig(cwd) { try { fs.writeFileSync(configPath, JSON.stringify(parsed, null, 2), 'utf-8'); } catch {} } + // Warn about unrecognized top-level keys so users don't silently lose config. + // Derived from config-set's VALID_CONFIG_KEYS (canonical source) plus internal-only + // keys that loadConfig handles but config-set doesn't expose. This avoids maintaining + // a hardcoded duplicate that drifts when new config keys are added. + const { VALID_CONFIG_KEYS } = require('./config.cjs'); + const KNOWN_TOP_LEVEL = new Set([ + // Extract top-level key names from dot-notation paths (e.g., 'workflow.research' → 'workflow') + ...[...VALID_CONFIG_KEYS].map(k => k.split('.')[0]), + // Section containers that hold nested sub-keys + 'git', 'workflow', 'planning', 'hooks', + // Internal keys loadConfig reads but config-set doesn't expose + 'model_overrides', 'agent_skills', 'context_window', 'resolve_model_ids', + // Deprecated keys (still accepted for migration, not in config-set) + 'depth', 'multiRepo', + ]); + const unknownKeys = Object.keys(parsed).filter(k => !KNOWN_TOP_LEVEL.has(k)); + if (unknownKeys.length > 0) { + process.stderr.write( + `gsd-tools: warning: unknown config key(s) in .planning/config.json: ${unknownKeys.join(', ')} — these will be ignored\n` + ); + } + const get = (key, nested) => { if (parsed[key] !== undefined) return parsed[key]; if (nested && parsed[nested.section] && parsed[nested.section][nested.field] !== undefined) { diff --git a/tests/core.test.cjs b/tests/core.test.cjs index 7dc61a33b..61dfad807 100644 --- a/tests/core.test.cjs +++ b/tests/core.test.cjs @@ -126,6 +126,61 @@ describe('loadConfig', () => { const config = loadConfig(tmpDir); assert.strictEqual(config.commit_docs, false); }); + + test('warns on unknown config keys to stderr (#1535)', () => { + writeConfig({ model_profile: 'quality', active_project: 'my-project', custom_flag: true }); + const origWrite = process.stderr.write; + let stderrOutput = ''; + process.stderr.write = (chunk) => { stderrOutput += chunk; }; + try { + const config = loadConfig(tmpDir); + // Known key still loads correctly + assert.strictEqual(config.model_profile, 'quality'); + // Warning emitted for unknown keys + assert.ok(stderrOutput.includes('active_project'), 'should warn about active_project'); + assert.ok(stderrOutput.includes('custom_flag'), 'should warn about custom_flag'); + assert.ok(stderrOutput.includes('ignored'), 'should mention keys will be ignored'); + } finally { + process.stderr.write = origWrite; + } + }); + + test('known config keys are derived from VALID_CONFIG_KEYS (not hardcoded)', () => { + // Verify that loadConfig's unknown-key check uses config-set's VALID_CONFIG_KEYS + // as its source of truth. If a new key is added to config-set, it should + // automatically be recognized by loadConfig without a separate update. + const { VALID_CONFIG_KEYS } = require('../get-shit-done/bin/lib/config.cjs'); + // Every top-level key from VALID_CONFIG_KEYS should be recognized + const topLevelKeys = [...VALID_CONFIG_KEYS].map(k => k.split('.')[0]); + for (const key of topLevelKeys) { + writeConfig({ [key]: 'test-value' }); + const origWrite = process.stderr.write; + let stderrOutput = ''; + process.stderr.write = (chunk) => { stderrOutput += chunk; }; + try { + loadConfig(tmpDir); + assert.ok( + !stderrOutput.includes(key), + `VALID_CONFIG_KEYS key "${key}" should not trigger unknown-key warning` + ); + } finally { + process.stderr.write = origWrite; + } + } + }); + + test('does not warn when all config keys are known', () => { + writeConfig({ model_profile: 'balanced', workflow: { research: false }, git: { branching_strategy: 'per-phase' } }); + const origWrite = process.stderr.write; + let stderrOutput = ''; + process.stderr.write = (chunk) => { stderrOutput += chunk; }; + try { + loadConfig(tmpDir); + assert.strictEqual(stderrOutput, '', 'should not emit any warnings for valid config'); + } finally { + process.stderr.write = origWrite; + } + }); }); // ─── loadConfig commit_docs gitignore auto-detection (#1250) ──────────────────