golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#2687): derive KNOWN_TOP_LEVEL from DYNAMIC_KEY_PATTERNS to eliminate read-side drift (#2706)

Browse Source 
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-04-25 12:08:59 -04:00
committed by GitHub
parent 94f8e895c0
commit b7ff14fe51
3 changed files with 143 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
get-shit-done/bin/lib/config-schema.cjs
View File

@@ -73,13 +73,13 @@ const VALID_CONFIG_KEYS = new Set([
* Each entry has a `test` function and a human-readable `description`.
*/
const DYNAMIC_KEY_PATTERNS = [
{ test: (k) => /^agent_skills\.[a-zA-Z0-9_-]+$/.test(k), description: 'agent_skills.<agent-type>' },
{ test: (k) => /^review\.models\.[a-zA-Z0-9_-]+$/.test(k), description: 'review.models.<cli-name>' },
{ test: (k) => /^features\.[a-zA-Z0-9_]+$/.test(k), description: 'features.<feature_name>' },
{ test: (k) => /^claude_md_assembly\.blocks\.[a-zA-Z0-9_]+$/.test(k), description: 'claude_md_assembly.blocks.<section>' },
{ topLevel: 'agent_skills', test: (k) => /^agent_skills\.[a-zA-Z0-9_-]+$/.test(k), description: 'agent_skills.<agent-type>' },
{ topLevel: 'review', test: (k) => /^review\.models\.[a-zA-Z0-9_-]+$/.test(k), description: 'review.models.<cli-name>' },
{ topLevel: 'features', test: (k) => /^features\.[a-zA-Z0-9_]+$/.test(k), description: 'features.<feature_name>' },
{ topLevel: 'claude_md_assembly', test: (k) => /^claude_md_assembly\.blocks\.[a-zA-Z0-9_]+$/.test(k), description: 'claude_md_assembly.blocks.<section>' },
// #2517 — runtime-aware model profile overrides: model_profile_overrides.<runtime>.<tier>
// <runtime> is a free string (so users can map non-built-in runtimes); <tier> is enum-restricted.
{ test: (k) => /^model_profile_overrides\.[a-zA-Z0-9_-]+\.(opus|sonnet|haiku)$/.test(k),
{ topLevel: 'model_profile_overrides', test: (k) => /^model_profile_overrides\.[a-zA-Z0-9_-]+\.(opus|sonnet|haiku)$/.test(k),
description: 'model_profile_overrides.<runtime>.<opus|sonnet|haiku>' },
];

11
get-shit-done/bin/lib/core.cjs
View File

@@ -336,14 +336,17 @@ function loadConfig(cwd) {
// 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');
// DYNAMIC_KEY_PATTERNS supplies topLevel for each pattern so adding a new
// dynamic-pattern namespace to config-schema.cjs automatically updates this set
// — no more drift between the read side and the write side (#2687).
const { VALID_CONFIG_KEYS, DYNAMIC_KEY_PATTERNS } = require('./config-schema.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', 'features',
// Dynamic-pattern top-level containers (e.g. review, model_profile_overrides)
...DYNAMIC_KEY_PATTERNS.map(p => p.topLevel),
// Internal keys loadConfig reads but config-set doesn't expose
'model_overrides', 'agent_skills', 'context_window', 'resolve_model_ids', 'claude_md_path',
'model_overrides', 'context_window', 'resolve_model_ids', 'claude_md_path',
// Deprecated keys (still accepted for migration, not in config-set)
'depth', 'multiRepo',
]);

131
tests/bug-2687-config-read-warning-parity.test.cjs Normal file
View File

@@ -0,0 +1,131 @@
'use strict';
/**
* Regression test for #2687 — loadConfig must not emit "unknown config key"
* warnings for keys that are registered in DYNAMIC_KEY_PATTERNS (e.g. review,
* model_profile_overrides, claude_md_assembly). These keys were absent from
* the hand-maintained KNOWN_TOP_LEVEL set in core.cjs, causing false-positive
* warnings on every read.
*
* We trigger loadConfig via `resolve-model` (which calls loadConfig internally).
* We use spawnSync to capture stderr from a process that exits 0 (warnings are
* written to stderr but don't cause a non-zero exit, so runGsdTools' error field
* is empty for successful commands).
*/
const { describe, test, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const { spawnSync } = require('node:child_process');
const { createTempProject, cleanup, TOOLS_PATH } = require('./helpers.cjs');
const TEST_ENV_BASE = {
GSD_SESSION_KEY: '',
CODEX_THREAD_ID: '',
CLAUDE_SESSION_ID: '',
CLAUDE_CODE_SSE_PORT: '',
OPENCODE_SESSION_ID: '',
GEMINI_SESSION_ID: '',
CURSOR_SESSION_ID: '',
WINDSURF_SESSION_ID: '',
TERM_SESSION_ID: '',
WT_SESSION: '',
TMUX_PANE: '',
ZELLIJ_SESSION_NAME: '',
TTY: '',
SSH_TTY: '',
};
/**
* Run gsd-tools and return { stdout, stderr, status }.
* Captures stderr even when the process exits 0 (unlike runGsdTools which only
* surfaces stderr via result.error on non-zero exit).
*/
function runWithStderr(args, cwd) {
const result = spawnSync(process.execPath, [TOOLS_PATH, ...args], {
cwd,
encoding: 'utf-8',
env: { ...process.env, ...TEST_ENV_BASE },
});
return {
stdout: result.stdout || '',
stderr: result.stderr || '',
status: result.status,
};
}
describe('bug-2687 — no warning for dynamic-pattern containers in loadConfig', () => {
let tmpDir;
afterEach(() => {
if (tmpDir) cleanup(tmpDir);
tmpDir = null;
});
test('review — loadConfig emits no warning when config.json contains review key', () => {
tmpDir = createTempProject('gsd-2687-review-');
const configPath = path.join(tmpDir, '.planning', 'config.json');
fs.writeFileSync(
configPath,
JSON.stringify({ review: { models: { 'test-cli': 'test-command' } } }, null, 2),
'utf-8'
);
// resolve-model calls loadConfig internally, triggering the KNOWN_TOP_LEVEL check
const result = runWithStderr(['resolve-model', 'planner'], tmpDir);
assert.ok(
!result.stderr.includes('unknown config key'),
`loadConfig must not warn about "review" — got stderr: ${result.stderr}`
);
assert.ok(
!result.stderr.includes('warning'),
`loadConfig must not warn about "review" — got stderr: ${result.stderr}`
);
});
test('model_profile_overrides — loadConfig emits no warning when config.json contains model_profile_overrides key', () => {
tmpDir = createTempProject('gsd-2687-mpo-');
const configPath = path.join(tmpDir, '.planning', 'config.json');
fs.writeFileSync(
configPath,
JSON.stringify({ model_profile_overrides: { codex: { sonnet: 'claude-sonnet-4' } } }, null, 2),
'utf-8'
);
// resolve-model calls loadConfig internally, triggering the KNOWN_TOP_LEVEL check
const result = runWithStderr(['resolve-model', 'planner'], tmpDir);
assert.ok(
!result.stderr.includes('unknown config key'),
`loadConfig must not warn about "model_profile_overrides" — got stderr: ${result.stderr}`
);
assert.ok(
!result.stderr.includes('warning'),
`loadConfig must not warn about "model_profile_overrides" — got stderr: ${result.stderr}`
);
});
test('claude_md_assembly — loadConfig emits no warning when config.json contains claude_md_assembly key', () => {
tmpDir = createTempProject('gsd-2687-cma-');
const configPath = path.join(tmpDir, '.planning', 'config.json');
fs.writeFileSync(
configPath,
JSON.stringify({ claude_md_assembly: { mode: 'custom', blocks: { identity: true } } }, null, 2),
'utf-8'
);
// resolve-model calls loadConfig internally, triggering the KNOWN_TOP_LEVEL check
const result = runWithStderr(['resolve-model', 'planner'], tmpDir);
assert.ok(
!result.stderr.includes('unknown config key'),
`loadConfig must not warn about "claude_md_assembly" — got stderr: ${result.stderr}`
);
assert.ok(
!result.stderr.includes('warning'),
`loadConfig must not warn about "claude_md_assembly" — got stderr: ${result.stderr}`
);
});
});