From 4fe072283dab5a498762dc48c952c60cd16bdc91 Mon Sep 17 00:00:00 2001 From: sim Date: Sat, 15 Aug 2026 01:38:52 -0400 Subject: [PATCH 1/4] test(#3532): failing-first suite for shadowed global-defaults diagnostic --- tests/config-loader.test.cjs | 148 +++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/tests/config-loader.test.cjs b/tests/config-loader.test.cjs index 8bf697425..b862e3f3a 100644 --- a/tests/config-loader.test.cjs +++ b/tests/config-loader.test.cjs @@ -1284,3 +1284,151 @@ describe('#2997: phase_id_convention is not silently dropped on a clean read', ( } finally { cleanup(tmpDir); } }); }); + +// ─── #3532 (10b): shadowed global-defaults diagnostic ───────────────────────── + +// The keys Branch D's _globalBaseCfg demonstrably honors when NO project config +// exists. Under a project .planning/config.json (Branch A — every real project) +// the global file is never opened, so each of these set globally is silently +// inert. `effort` is deliberately absent: the install-time effort sync +// (readGsdEffectiveEffortConfig) DOES merge the global file, so warning on it +// would be false for the channel users control via effort sync. +const GLOBAL_KEYS_SHADOWED_UNDER_PROJECT = [ + 'model_profile', 'commit_docs', 'research', 'plan_checker', 'verifier', + 'nyquist_validation', 'post_planning_gaps', 'parallelization', 'text_mode', + 'resolve_model_ids', 'context_window', 'subagent_timeout', 'model_overrides', + 'models', 'granularity', 'granularities', 'planning', 'dynamic_routing', + 'fast_mode', 'agent_skills', 'response_language', 'runtime', + 'model_profile_overrides', 'model_policy', +]; + +describe('#3532 shadowed global-defaults warning', () => { + let tmpDir; + let gsdHome; + let stderrLines; + let originalStderrWrite; + let originalGsdHome; + + beforeEach(() => { + tmpDir = makeTempProject('gsd-3532-shadow-'); + gsdHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3532-home-')); + stderrLines = []; + originalStderrWrite = process.stderr.write.bind(process.stderr); + process.stderr.write = (chunk) => { stderrLines.push(String(chunk)); return true; }; + originalGsdHome = process.env.GSD_HOME; + process.env.GSD_HOME = gsdHome; + if (_resetRuntimeWarningCacheForTests) _resetRuntimeWarningCacheForTests(); + }); + + afterEach(() => { + process.stderr.write = originalStderrWrite; + if (originalGsdHome === undefined) delete process.env.GSD_HOME; + else process.env.GSD_HOME = originalGsdHome; + if (tmpDir) cleanup(tmpDir); + if (gsdHome) cleanup(gsdHome); + tmpDir = gsdHome = null; + }); + + function writeGlobalDefaults(obj) { + fs.mkdirSync(path.join(gsdHome, '.gsd'), { recursive: true }); + fs.writeFileSync( + path.join(gsdHome, '.gsd', 'defaults.json'), + JSON.stringify(obj, null, 2), + ); + } + + test('project config + global model keys -> one warning naming both keys', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + writeGlobalDefaults({ model_overrides: { 'gsd-executor': 'haiku' }, model_profile: 'quality' }); + loadConfigResolved(tmpDir); + const warnings = stderrLines.filter(l => l.includes('model_overrides') && l.includes('model_profile')); + assert.equal(warnings.length, 1, `expected exactly one shadowed-keys warning, got: ${stderrLines.join('')}`); + }); + + test('second loadConfig call does not repeat the warning', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + writeGlobalDefaults({ model_profile: 'quality' }); + loadConfigResolved(tmpDir); + loadConfigResolved(tmpDir); + const warnings = stderrLines.filter(l => l.includes('model_profile') && l.includes('defaults.json')); + assert.ok(warnings.length <= 1, `warning emitted more than once: ${warnings.length}`); + }); + + test('global effort keys do not warn (honored by the install-time effort sync)', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + writeGlobalDefaults({ effort: { default: 'low' } }); + loadConfigResolved(tmpDir); + assert.equal(stderrLines.filter(l => l.includes('defaults.json')).length, 0, + `effort must not trigger the shadow warning: ${stderrLines.join('')}`); + }); + + test('absent global defaults never warn', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + loadConfigResolved(tmpDir); + assert.equal(stderrLines.length, 0, `unexpected warnings: ${stderrLines.join('')}`); + }); + + test('bare dir without .planning honors global defaults without warning (Branch D)', () => { + const bare = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3532-bare-')); + try { + writeGlobalDefaults({ model_profile: 'quality' }); + const resolution = loadConfigResolved(bare); + assert.equal(resolution.source, 'global-defaults'); + assert.equal(resolution.config['model_profile'], 'quality'); + assert.equal(stderrLines.length, 0, `Branch D must not warn: ${stderrLines.join('')}`); + } finally { + cleanup(bare); + } + }); + + test('unparseable global defaults skip the shadow warning', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + fs.mkdirSync(path.join(gsdHome, '.gsd'), { recursive: true }); + fs.writeFileSync(path.join(gsdHome, '.gsd', 'defaults.json'), '{not json'); + loadConfigResolved(tmpDir); + assert.equal(stderrLines.filter(l => l.includes('shadowed')).length, 0); + }); + + test('present-but-empty project config still shadows', () => { + writeConfig(tmpDir, {}); + writeGlobalDefaults({ model_profile: 'quality' }); + loadConfigResolved(tmpDir); + assert.ok(stderrLines.some(l => l.includes('model_profile')), + `empty project config must still warn: ${stderrLines.join('')}`); + }); + + test('non-resolution global keys do not warn from this check', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + writeGlobalDefaults({ __gsd_3532_arbitrary__: true }); + loadConfigResolved(tmpDir); + assert.equal(stderrLines.filter(l => l.includes('__gsd_3532_arbitrary__') && l.includes('shadowed')).length, 0); + }); + + // Parity canary: every key Branch D honors must warn when set globally under + // a project config. If _globalBaseCfg grows a key this list misses, the + // warning goes silent for it; if this list grows a key _globalBaseCfg does + // not read, the warning lies. Both directions fail here first. + for (const key of GLOBAL_KEYS_SHADOWED_UNDER_PROJECT) { + test(`canary: global "${key}" alone warns under a project config`, () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + writeGlobalDefaults({ [key]: true }); + loadConfigResolved(tmpDir); + assert.ok( + stderrLines.some(l => l.includes(key)), + `global "${key}" must be reported as shadowed; stderr: ${stderrLines.join('')}`, + ); + }); + } + + test('_resetRuntimeWarningCacheForTests clears the shadowed-key dedup set', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + writeGlobalDefaults({ model_profile: 'quality' }); + loadConfigResolved(tmpDir); + assert.ok( + configLoader._warnedShadowedGlobalKeys && configLoader._warnedShadowedGlobalKeys.size > 0, + 'precondition: a shadowed key must populate the dedup set', + ); + _resetRuntimeWarningCacheForTests(); + assert.equal(configLoader._warnedShadowedGlobalKeys.size, 0); + }); +}); From 129871a8be0f0f9b21993e2555937cf4e413dc26 Mon Sep 17 00:00:00 2001 From: sim Date: Sat, 15 Aug 2026 01:48:41 -0400 Subject: [PATCH 2/4] fix(#3532): warn when global defaults keys are shadowed by a project config --- .changeset/graceful-seals-march.md | 5 +++ docs/CONFIGURATION.md | 16 ++++++++ src/config-loader.cts | 61 ++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 .changeset/graceful-seals-march.md diff --git a/.changeset/graceful-seals-march.md b/.changeset/graceful-seals-march.md new file mode 100644 index 000000000..8ca8ea838 --- /dev/null +++ b/.changeset/graceful-seals-march.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 0 +--- +**`~/.gsd/defaults.json` shadowing is now diagnosed instead of silent** — in any project with a `.planning/config.json`, global model-side keys (`model_profile`, `model_overrides`, `models`, `dynamic_routing`, `runtime`, …) were silently ignored for model resolution; a file named `defaults.json` applied to no real project with no signal. GSD now prints a one-time stderr warning naming the shadowed keys. Resolution precedence is unchanged; global `effort` keeps working via effort sync and never warns. (#3532) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index a3bc1cd34..7004e45c6 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -1744,6 +1744,22 @@ Save settings as global defaults for future projects: When `/gsd-new-project` creates a new `config.json`, it reads global defaults and merges them as the starting configuration. Per-project settings always override globals. +### What a global file can and cannot set at runtime + +Two different rules apply, and the difference is deliberate ([#3532](https://github.com/open-gsd/gsd-core/issues/3532)): + +- **In a directory with no `.planning/` at all**, `~/.gsd/defaults.json` is the active + configuration — model resolution reads it directly. +- **In a real project (`.planning/config.json` present, even if empty)**, the global file is + **not read for model resolution** — every model-side key it sets (`model_profile`, + `model_overrides`, `models`, `dynamic_routing`, `runtime`, and the rest of the resolution + set) is inert there. GSD prints a one-time stderr warning naming the shadowed keys when it + detects this, instead of failing silently. To apply a global model setting to a project, + put it in that project's `.planning/config.json`. +- **`effort` is the exception**: the install-time effort channel always merges + `~/.gsd/defaults.json` with the project config (that is how `effort sync` works), so a + global `effort` block keeps working in projects and does not trigger the warning. + --- ## Observability diff --git a/src/config-loader.cts b/src/config-loader.cts index cf0a95594..60b0e5138 100644 --- a/src/config-loader.cts +++ b/src/config-loader.cts @@ -317,6 +317,49 @@ function _resetRuntimeWarningCacheForTests(): void { _warnedConfigKeys.clear(); _warnedUnknownConfigKeys.clear(); _warnedUnusableConfig.clear(); + _warnedShadowedGlobalKeys.clear(); +} + +// ─── #3532 (10b): shadowed global-defaults diagnostic ──────────────────────── + +// The keys Branch D's `_globalBaseCfg` demonstrably honors from +// ~/.gsd/defaults.json when no project config exists. Under a project +// .planning/config.json (Branch A — every real project) the global file is +// never opened, so each of these set globally is silently inert for resolution. +// `effort` is in Branch D's honored set but is EXCLUDED from the shadow warning: +// the install-time effort sync (readGsdEffectiveEffortConfig) DOES merge the +// global file, so warning on it would be false for the channel users actually +// control via `effort sync`. Keep this list in lockstep with `_globalBaseCfg` +// below — the per-key canary in tests/config-loader.test.cjs fails first on +// drift in either direction. +const GLOBAL_DEFAULTS_RESOLUTION_KEYS = [ + 'model_profile', 'commit_docs', 'research', 'plan_checker', 'verifier', + 'nyquist_validation', 'post_planning_gaps', 'parallelization', 'text_mode', + 'resolve_model_ids', 'context_window', 'subagent_timeout', 'model_overrides', + 'models', 'granularity', 'granularities', 'planning', 'dynamic_routing', + 'effort', 'fast_mode', 'agent_skills', 'response_language', 'runtime', + 'model_profile_overrides', 'model_policy', +]; + +// Module-level dedup keyed on the sorted shadowed-key set: a later call with +// the same shadowed set stays quiet, while a config that grows a new shadowed +// key re-arms the warning (same discipline as _warnedUnknownConfigKeys). +const _warnedShadowedGlobalKeys = new Set(); + +function _warnShadowedGlobalDefaults(globalDefaults: Record, globalPath: string): void { + const shadowed = GLOBAL_DEFAULTS_RESOLUTION_KEYS.filter(k => + k !== 'effort' && Object.prototype.hasOwnProperty.call(globalDefaults, k)); + if (shadowed.length === 0) return; + const dedupKey = shadowed.slice().sort().join(','); + if (_warnedShadowedGlobalKeys.has(dedupKey)) return; + _warnedShadowedGlobalKeys.add(dedupKey); + try { + process.stderr.write( + `gsd-tools: warning: ${globalPath} sets ${shadowed.join(', ')} but this project's ` + + `.planning/config.json takes precedence — those global keys are ignored for model ` + + `resolution here. (#3532)\n`, + ); + } catch { /* stderr might be closed in some test harnesses */ } } // ─── FIX 2: Federated overlay helpers ──────────────────────────────────────── @@ -836,6 +879,22 @@ function loadConfigResolved(cwd: string, options: Record = {}): // Fix 4: empty-string ws ('') resolves the root path → source:'root'. const source: ConfigSource = wsRequested ? 'workstream' : 'root'; + // #3532 (10b): a parsed project config means Branch D never runs, so every + // key ~/.gsd/defaults.json sets that Branch D would honor is silently inert + // here. Observation only — one deduped stderr warning; precedence is + // untouched. Faults in the global file stay silent in this branch (the + // project config governs; the nearer file is the actionable one). + try { + const shadowHome = process.env['GSD_HOME'] || os.homedir(); + const shadowPath = path.join(shadowHome, '.gsd', 'defaults.json'); + const shadowRead = _readConfigFile(shadowPath); + if (shadowRead.kind === 'ok') { + _warnShadowedGlobalDefaults(shadowRead.data, shadowPath); + } + } catch { + // Observation only — never let the diagnostic perturb resolution. + } + // This config parsed — but a DIFFERENT file on the resolution path may not // have. A workstream config that loads cleanly while the root config it // inherits from is corrupt is still a degraded resolution: the root's @@ -965,6 +1024,8 @@ export = { _getNestedConfigDefault, _deepMergeConfig, _warnedUnknownConfigKeys, + _warnedShadowedGlobalKeys, + GLOBAL_DEFAULTS_RESOLUTION_KEYS, _warnUnknownProfileOverrides, _resetRuntimeWarningCacheForTests, _warnedConfigKeys, From a280054040adc2b8014fb0d50a25b2c1ad933d92 Mon Sep 17 00:00:00 2001 From: sim Date: Sat, 15 Aug 2026 01:56:37 -0400 Subject: [PATCH 3/4] fix(#3532): hermetic child GSD_HOME, typed-IR canaries, nested alias, list parity --- src/config-loader.cts | 21 +++++++++++++----- tests/config-loader.test.cjs | 43 +++++++++++++++++++++++++++--------- 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/config-loader.cts b/src/config-loader.cts index 60b0e5138..aa7d59fe4 100644 --- a/src/config-loader.cts +++ b/src/config-loader.cts @@ -341,23 +341,34 @@ const GLOBAL_DEFAULTS_RESOLUTION_KEYS = [ 'model_profile_overrides', 'model_policy', ]; -// Module-level dedup keyed on the sorted shadowed-key set: a later call with +// Module-level dedup keyed on the SORTED shadowed-key set: a later call with // the same shadowed set stays quiet, while a config that grows a new shadowed -// key re-arms the warning (same discipline as _warnedUnknownConfigKeys). +// key re-arms the warning. Stronger than _warnedUnknownConfigKeys (which keys +// on insertion order) — same discipline, order-independent key. const _warnedShadowedGlobalKeys = new Set(); function _warnShadowedGlobalDefaults(globalDefaults: Record, globalPath: string): void { const shadowed = GLOBAL_DEFAULTS_RESOLUTION_KEYS.filter(k => k !== 'effort' && Object.prototype.hasOwnProperty.call(globalDefaults, k)); + // Branch D also honors the nested alias workflow.post_planning_gaps (the + // `?? globalDefaults['workflow']?.['post_planning_gaps']` fallback in + // _globalBaseCfg) — a global file using only the nested form is equally + // shadowed, so it reports under its dotted name. + if (!shadowed.includes('post_planning_gaps')) { + const wf = globalDefaults['workflow']; + if (wf && typeof wf === 'object' && !Array.isArray(wf) && + Object.prototype.hasOwnProperty.call(wf, 'post_planning_gaps')) { + shadowed.push('workflow.post_planning_gaps'); + } + } if (shadowed.length === 0) return; const dedupKey = shadowed.slice().sort().join(','); if (_warnedShadowedGlobalKeys.has(dedupKey)) return; _warnedShadowedGlobalKeys.add(dedupKey); try { process.stderr.write( - `gsd-tools: warning: ${globalPath} sets ${shadowed.join(', ')} but this project's ` + - `.planning/config.json takes precedence — those global keys are ignored for model ` + - `resolution here. (#3532)\n`, + `gsd-tools: warning: ${globalPath} sets ${shadowed.join(', ')} but a project config ` + + `takes precedence here — those global keys are ignored for model resolution. (#3532)\n`, ); } catch { /* stderr might be closed in some test harnesses */ } } diff --git a/tests/config-loader.test.cjs b/tests/config-loader.test.cjs index b862e3f3a..edb1c13ec 100644 --- a/tests/config-loader.test.cjs +++ b/tests/config-loader.test.cjs @@ -740,7 +740,7 @@ const { describe, test, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { createTempProject, cleanup, TOOLS_PATH, TEST_ENV_BASE } = require('./helpers.cjs'); +const { createTempProject, cleanup, TOOLS_PATH, TEST_ENV_BASE, installSpawnHome } = require('./helpers.cjs'); const { runNode } = require('./helpers/process-seam.cjs'); const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); @@ -751,7 +751,11 @@ const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); function runWithStderr(args, cwd, env = {}) { const result = runNode([TOOLS_PATH, ...args], { cwd, - env: { ...process.env, ...TEST_ENV_BASE, ...env }, + // #3532: pin GSD_HOME to an empty sandbox so a developer's real + // ~/.gsd/defaults.json cannot leak shadow-key warnings into children that + // these suites assert are stderr-clean (TEST_ENV_BASE only BLANKS the + // var; an empty string falls through to the real homedir). + env: { ...process.env, ...TEST_ENV_BASE, GSD_HOME: installSpawnHome(), ...env }, timeoutMs: PROBE_TIMEOUT_MS, }); return { @@ -1404,22 +1408,41 @@ describe('#3532 shadowed global-defaults warning', () => { assert.equal(stderrLines.filter(l => l.includes('__gsd_3532_arbitrary__') && l.includes('shadowed')).length, 0); }); - // Parity canary: every key Branch D honors must warn when set globally under - // a project config. If _globalBaseCfg grows a key this list misses, the - // warning goes silent for it; if this list grows a key _globalBaseCfg does - // not read, the warning lies. Both directions fail here first. + // Typed-IR parity canary (CONTRIBUTING: assert the exported dedup Set, not + // stderr prose — #2674 precedent). Every key Branch D honors must register + // as shadowed when set globally under a project config. for (const key of GLOBAL_KEYS_SHADOWED_UNDER_PROJECT) { test(`canary: global "${key}" alone warns under a project config`, () => { writeConfig(tmpDir, { model_profile: 'balanced' }); writeGlobalDefaults({ [key]: true }); loadConfigResolved(tmpDir); - assert.ok( - stderrLines.some(l => l.includes(key)), - `global "${key}" must be reported as shadowed; stderr: ${stderrLines.join('')}`, - ); + const registered = [...configLoader._warnedShadowedGlobalKeys].some(set => set.split(',').includes(key)); + assert.ok(registered, `global "${key}" must register as shadowed`); }); } + // The nested alias Branch D honors (workflow.post_planning_gaps fallback in + // _globalBaseCfg) is equally shadowed and reports under its dotted name. + test('canary: nested workflow.post_planning_gaps warns under a project config', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + writeGlobalDefaults({ workflow: { post_planning_gaps: 'extended' } }); + loadConfigResolved(tmpDir); + const registered = [...configLoader._warnedShadowedGlobalKeys].some(set => set.split(',').includes('workflow.post_planning_gaps')); + assert.ok(registered, 'nested workflow.post_planning_gaps must register as shadowed'); + }); + + // List parity, both directions: the implementation's exported list (minus + // effort) must equal this file's expected list — a key _globalBaseCfg grows + // without updating GLOBAL_DEFAULTS_RESOLUTION_KEYS goes silently unwarned, + // and a key the export grows without _globalBaseCfg reading makes the + // warning lie. + test('GLOBAL_DEFAULTS_RESOLUTION_KEYS parity with the expected shadow set', () => { + const exported = configLoader.GLOBAL_DEFAULTS_RESOLUTION_KEYS.filter(k => k !== 'effort').sort(); + const expected = GLOBAL_KEYS_SHADOWED_UNDER_PROJECT.slice().sort(); + assert.deepEqual(exported, expected, + `resolution-key list drifted: exported=${JSON.stringify(exported)} expected=${JSON.stringify(expected)}`); + }); + test('_resetRuntimeWarningCacheForTests clears the shadowed-key dedup set', () => { writeConfig(tmpDir, { model_profile: 'balanced' }); writeGlobalDefaults({ model_profile: 'quality' }); From b317af347006cae8f5d72d7f3037c7dad041da94 Mon Sep 17 00:00:00 2001 From: sim Date: Sat, 15 Aug 2026 02:07:15 -0400 Subject: [PATCH 4/4] chore(#3532): backfill changeset pr number --- .changeset/graceful-seals-march.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/graceful-seals-march.md b/.changeset/graceful-seals-march.md index 8ca8ea838..3cff75ea8 100644 --- a/.changeset/graceful-seals-march.md +++ b/.changeset/graceful-seals-march.md @@ -1,5 +1,5 @@ --- type: Added -pr: 0 +pr: 3540 --- **`~/.gsd/defaults.json` shadowing is now diagnosed instead of silent** — in any project with a `.planning/config.json`, global model-side keys (`model_profile`, `model_overrides`, `models`, `dynamic_routing`, `runtime`, …) were silently ignored for model resolution; a file named `defaults.json` applied to no real project with no signal. GSD now prints a one-time stderr warning naming the shadowed keys. Resolution precedence is unchanged; global `effort` keeps working via effort sync and never warns. (#3532)