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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -440,6 +440,7 @@ function getCmdConfigSetModelProfileResultMessage(
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
VALID_CONFIG_KEYS,
|
||||
cmdConfigEnsureSection,
|
||||
cmdConfigSet,
|
||||
cmdConfigGet,
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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) ──────────────────
|
||||
|
||||
Reference in New Issue
Block a user