From e4ed71bf7917df585905a63a243af50d74fbbcd3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 14 May 2026 19:03:31 -0400 Subject: [PATCH 1/5] =?UTF-8?q?test(3523):=20red=20=E2=80=94=20legacy=20to?= =?UTF-8?q?p-level=20branching=5Fstrategy=20warning=20+=20migration=20+=20?= =?UTF-8?q?parity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six failing tests covering all Done-when criteria from #3523: 1. No warning emitted for top-level branching_strategy 2. Value still surfaced via git.branching_strategy after loadConfig 3. Double-emission capped to single-emission per process 4-5. On-disk migration (option 3): write-back + no-clobber guard 6. CJS↔SDK contract: both agree on legacy-shape fixture Co-Authored-By: Claude Sonnet 4.6 --- ...config-branching-strategy-warning.test.cjs | 315 ++++++++++++++++++ 1 file changed, 315 insertions(+) create mode 100644 tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs diff --git a/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs b/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs new file mode 100644 index 000000000..4ffc84ed3 --- /dev/null +++ b/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs @@ -0,0 +1,315 @@ +'use strict'; + +/** + * Regression tests for #3523 — CJS loadConfig must not emit a false + * "unknown config key(s)" warning for `branching_strategy` when that key + * is written at the top level of .planning/config.json. + * + * Root cause: KNOWN_TOP_LEVEL in core.cjs was built from VALID_CONFIG_KEYS + * via k.split('.')[0], which turns 'git.branching_strategy' → 'git', not + * 'branching_strategy'. So a config with the legacy top-level shape tripped + * the unknown-key warning even though core.cjs:485 actively reads the value. + * + * Fix (option 3 — self-healing): mirror the multiRepo → planning.sub_repos + * precedent: graft branching_strategy into fileData.git.branching_strategy + * and delete the top-level key, then persist. The KNOWN_TOP_LEVEL list also + * gains 'branching_strategy' as a deprecated-still-accepted key so the warning + * never fires even on the first read before the write-back occurs. + * + * Double-emission is also reduced: the warning site is guarded by a + * module-level Set so repeated loadConfig calls during one CLI invocation + * don't echo the same line twice. + * + * CJS↔SDK contract: the SDK mergeDefaults() already handles the legacy + * top-level key (PR #3116). This file adds a fixture-level parity check + * that proves both paths produce the same branching_strategy value. + * + * Test strategy: we use `resolve-model` as the minimal CJS entry point that + * calls loadConfig internally, then assert on stderr emptiness (typed-IR + * "no warning" pattern from #2687). + */ + +const { describe, test, afterEach, before } = 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 }. + * Always captures stderr even when exit code is 0. + */ +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, + }; +} + +// ─── Test 1: no warning for legacy top-level branching_strategy ────────────── + +describe('bug-3523 — no warning for legacy top-level branching_strategy', () => { + let tmpDir; + + afterEach(() => { + if (tmpDir) cleanup(tmpDir); + tmpDir = null; + }); + + test('loadConfig emits no stderr when config.json has top-level branching_strategy', () => { + tmpDir = createTempProject('gsd-3523-warn-'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + fs.writeFileSync( + configPath, + JSON.stringify({ + branching_strategy: 'phase', + git: { base_branch: 'main' }, + }, null, 2), + 'utf-8' + ); + + // resolve-model calls loadConfig internally, triggering KNOWN_TOP_LEVEL check. + const result = runWithStderr(['resolve-model', 'planner'], tmpDir); + + assert.equal( + result.stderr.trim(), + '', + `loadConfig must not warn about top-level branching_strategy (#3523) — got: ${result.stderr}` + ); + }); + + test('branching_strategy value is still surfaced after loadConfig on legacy shape', () => { + tmpDir = createTempProject('gsd-3523-value-'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + fs.writeFileSync( + configPath, + JSON.stringify({ + branching_strategy: 'milestone', + git: { base_branch: 'main' }, + }, null, 2), + 'utf-8' + ); + + // Trigger loadConfig (which runs the migration and writes git.branching_strategy + // back to disk), then read it with config-get to verify the value is preserved. + const triggerResult = runWithStderr(['resolve-model', 'planner'], tmpDir); + assert.equal( + triggerResult.stderr.trim(), + '', + `No warning should fire on legacy shape (#3523) — got: ${triggerResult.stderr}` + ); + + // After migration write-back, config-get should find git.branching_strategy. + const result = runWithStderr(['config-get', 'git.branching_strategy'], tmpDir); + + assert.equal( + result.stderr.trim(), + '', + `No error should fire when reading migrated branching_strategy (#3523) — got: ${result.stderr}` + ); + assert.ok( + result.stdout.includes('milestone'), + `Expected git.branching_strategy to be 'milestone' but got: ${result.stdout}` + ); + }); +}); + +// ─── Test 2: no duplicated warning (double-emission) ───────────────────────── + +describe('bug-3523 — double-emission reduced to single-emission', () => { + let tmpDir; + + afterEach(() => { + if (tmpDir) cleanup(tmpDir); + tmpDir = null; + }); + + test('unknown-key warning appears at most once per process invocation', () => { + // Use a key that IS genuinely unknown (not branching_strategy, which is now + // fixed) to verify the deduplication guard works for other keys too. + // We verify that the count of warning lines for a single unknown key is + // at most 1 — not 2 — even if loadConfig is invoked twice internally. + tmpDir = createTempProject('gsd-3523-dedup-'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + fs.writeFileSync( + configPath, + JSON.stringify({ + // intentionally_unknown_key_for_dedup_test: a key that can never be valid + __gsd3523_dedup_sentinel__: true, + }, null, 2), + 'utf-8' + ); + + const result = runWithStderr(['resolve-model', 'planner'], tmpDir); + + // Count how many times the sentinel key appears in warnings + const warningLines = result.stderr + .split('\n') + .filter(l => l.includes('__gsd3523_dedup_sentinel__')); + + assert.ok( + warningLines.length <= 1, + `Unknown-key warning must appear at most once per process invocation — ` + + `appeared ${warningLines.length} times. stderr:\n${result.stderr}` + ); + }); +}); + +// ─── Test 3: on-disk migration (option 3 write-back) ───────────────────────── + +describe('bug-3523 — option 3 on-disk migration of branching_strategy', () => { + let tmpDir; + + afterEach(() => { + if (tmpDir) cleanup(tmpDir); + tmpDir = null; + }); + + test('after loadConfig, on-disk config.json has branching_strategy under git.*', () => { + tmpDir = createTempProject('gsd-3523-writeback-'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + fs.writeFileSync( + configPath, + JSON.stringify({ + branching_strategy: 'phase', + git: { base_branch: 'main' }, + }, null, 2), + 'utf-8' + ); + + // Trigger loadConfig by running a command. + runWithStderr(['resolve-model', 'planner'], tmpDir); + + // On-disk file should now have git.branching_strategy and no top-level branching_strategy. + const onDisk = JSON.parse(fs.readFileSync(configPath, 'utf-8')); + assert.equal( + onDisk.git?.branching_strategy, + 'phase', + 'Expected on-disk config.json to have git.branching_strategy = "phase" after migration' + ); + assert.equal( + onDisk.branching_strategy, + undefined, + 'Expected on-disk config.json to have no top-level branching_strategy after migration' + ); + }); + + test('migration does not clobber existing git.branching_strategy', () => { + // If git.branching_strategy is already set, the top-level value should + // not overwrite it (nested wins, matching SDK mergeDefaults precedence). + tmpDir = createTempProject('gsd-3523-no-clobber-'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + fs.writeFileSync( + configPath, + JSON.stringify({ + branching_strategy: 'phase', // legacy top-level + git: { + base_branch: 'main', + branching_strategy: 'milestone', // canonical nested — must win + }, + }, null, 2), + 'utf-8' + ); + + // Trigger loadConfig. + runWithStderr(['resolve-model', 'planner'], tmpDir); + + const onDisk = JSON.parse(fs.readFileSync(configPath, 'utf-8')); + assert.equal( + onDisk.git?.branching_strategy, + 'milestone', + 'canonical git.branching_strategy must not be overwritten by legacy top-level key' + ); + // top-level key should be removed since it was redundant + assert.equal( + onDisk.branching_strategy, + undefined, + 'top-level branching_strategy should be removed even when git.branching_strategy already set' + ); + }); +}); + +// ─── Test 4: CJS↔SDK contract parity ──────────────────────────────────────── + +describe('bug-3523 — CJS↔SDK contract: both agree on legacy branching_strategy fixture', () => { + /** + * This is a light-touch contract test: we invoke the CJS path via CLI and + * compare the branching_strategy value it returns against what the SDK's + * mergeDefaults would compute for the same fixture. + * + * We can't import SDK TypeScript here, so we assert on the CJS output and + * use a snapshot of expected SDK behavior derived from the mergeDefaults + * source (sdk/src/config.ts:192-218): + * mergeDefaults({ branching_strategy: 'phase', git: { base_branch: 'main' } }) + * → git.branching_strategy = 'phase' + */ + let tmpDir; + + afterEach(() => { + if (tmpDir) cleanup(tmpDir); + tmpDir = null; + }); + + test('CJS loadConfig surfaces branching_strategy matching SDK mergeDefaults behavior', () => { + tmpDir = createTempProject('gsd-3523-parity-'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + // The fixture that the SDK's mergeDefaults handles correctly (PR #3116). + fs.writeFileSync( + configPath, + JSON.stringify({ + branching_strategy: 'phase', + git: { base_branch: 'main' }, + }, null, 2), + 'utf-8' + ); + + // SDK mergeDefaults produces: git.branching_strategy = 'phase' + // CJS loadConfig must produce the same. Trigger loadConfig first (migration + // writes git.branching_strategy to disk), then verify with config-get. + const triggerResult = runWithStderr(['resolve-model', 'planner'], tmpDir); + assert.equal( + triggerResult.stderr.trim(), + '', + `No warning must fire on a standard legacy fixture — got: ${triggerResult.stderr}` + ); + + // After the migration write-back, config-get must find git.branching_strategy = 'phase', + // matching what the SDK's mergeDefaults would compute. + const result = runWithStderr(['config-get', 'git.branching_strategy'], tmpDir); + + assert.equal( + result.stderr.trim(), + '', + `No error when reading post-migration git.branching_strategy — got: ${result.stderr}` + ); + assert.ok( + result.stdout.includes('phase'), + `CJS must agree with SDK: git.branching_strategy = 'phase' for legacy fixture. ` + + `Got: ${result.stdout}` + ); + }); +}); From 3ab24c6c569186e6d7dd6238ce8bbcb1132b7e7e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 14 May 2026 19:03:39 -0400 Subject: [PATCH 2/5] fix(3523): self-healing migration of legacy top-level branching_strategy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three-part fix for the false "unknown config key(s)" warning fired for top-level `branching_strategy` in .planning/config.json: 1. On-disk migration (option 3, mirroring multiRepo → planning.sub_repos): When loadConfig reads a config.json with top-level `branching_strategy` set and `git.branching_strategy` unset, it grafts the value into `git.branching_strategy` and deletes the top-level key, then persists. If `git.branching_strategy` is already set, the nested value wins (matches SDK mergeDefaults precedence, PR #3116). 2. KNOWN_TOP_LEVEL safety net: 'branching_strategy' added to the deprecated- keys bucket so the warning never fires even on the first read of a root config that feeds a workstream merge (where `parsed` may still carry it). 3. Double-emission guard: a module-level `_warnedUnknownConfigKeys` Set deduplicates the unknown-key warning across multiple loadConfig calls within a single CLI invocation (init phase-op N called it twice). Closes #3523 Co-Authored-By: Claude Sonnet 4.6 --- get-shit-done/bin/lib/core.cjs | 38 ++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 98c62630f..6454da631 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -399,6 +399,21 @@ function loadConfig(cwd, options = {}) { configDirty = true; } + // #3523 — Migrate legacy top-level branching_strategy → git.branching_strategy. + // Canonical location is git.branching_strategy (per config-schema.cjs); writing + // at the top level trips the unknown-key warning even though loadConfig:485 actively + // reads it via the nested fallback. This migration mirrors the multiRepo → sub_repos + // precedent: graft then delete so the warning never fires again on this project. + // The nested value wins if already set (matches SDK mergeDefaults precedence, PR #3116). + if (Object.prototype.hasOwnProperty.call(fileData, 'branching_strategy')) { + if (!fileData.git) fileData.git = {}; + if (!fileData.git.branching_strategy) { + fileData.git.branching_strategy = fileData.branching_strategy; + } + delete fileData.branching_strategy; + configDirty = true; + } + // Keep planning.sub_repos in sync with actual filesystem const currentSubRepos = fileData.planning?.sub_repos || []; if (Array.isArray(currentSubRepos) && currentSubRepos.length > 0) { @@ -439,13 +454,23 @@ function loadConfig(cwd, options = {}) { // Internal keys loadConfig reads but config-set doesn't expose 'model_overrides', 'context_window', 'resolve_model_ids', 'claude_md_path', // Deprecated keys (still accepted for migration, not in config-set) - 'depth', 'multiRepo', + // 'branching_strategy' is kept here as a safety net: it is migrated to + // git.branching_strategy above (#3523), but on the first read of a root + // config that feeds into a workstream merge, `parsed` may still surface it. + 'depth', 'multiRepo', 'branching_strategy', ]); 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` - ); + // Deduplicate: a single `init phase-op N` invocation calls loadConfig twice + // (once for the sub-command setup, once for git-config resolution). Guard with + // a module-level Set so the same message never fires more than once per process. + const warnKey = unknownKeys.join(','); + if (!_warnedUnknownConfigKeys.has(warnKey)) { + _warnedUnknownConfigKeys.add(warnKey); + process.stderr.write( + `gsd-tools: warning: unknown config key(s) in .planning/config.json: ${unknownKeys.join(', ')} — these will be ignored\n` + ); + } } // #2517 — Validate runtime/tier values for keys that loadConfig handles but @@ -585,6 +610,11 @@ function loadConfig(cwd, options = {}) { // ─── Git utilities ──────────────────────────────────────────────────────────── +// Module-level deduplication for unknown-key warnings (#3523). +// A single `init phase-op N` call invokes loadConfig more than once; this Set +// prevents the same warning from being echoed on each invocation. +const _warnedUnknownConfigKeys = new Set(); + const _gitIgnoredCache = new Map(); function isGitIgnored(cwd, targetPath) { From 0f7b018d4dc1b453b11d76eecea3f100c2335619 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 14 May 2026 19:03:42 -0400 Subject: [PATCH 3/5] chore: changeset for fix/3523 (branching_strategy false warning) Co-Authored-By: Claude Sonnet 4.6 --- .changeset/sunny-pandas-dance.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/sunny-pandas-dance.md diff --git a/.changeset/sunny-pandas-dance.md b/.changeset/sunny-pandas-dance.md new file mode 100644 index 000000000..782c550d0 --- /dev/null +++ b/.changeset/sunny-pandas-dance.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3523 +--- +**Top-level `branching_strategy` in `.planning/config.json` no longer triggers a false "unknown config key" warning** — `loadConfig` in `core.cjs` actively read the key via its nested fallback (`git.branching_strategy`) but the `KNOWN_TOP_LEVEL` allowlist was built from dot-notation paths via `.split('.')[0]`, turning `'git.branching_strategy'` into `'git'` instead of `'branching_strategy'`. The fix adds a self-healing on-disk migration (mirroring the existing `multiRepo → planning.sub_repos` precedent): on first `loadConfig`, the top-level key is grafted into `git.branching_strategy` and the stale top-level entry is removed. A module-level deduplication Set also prevents the same unknown-key warning from appearing twice when `loadConfig` is invoked multiple times in a single CLI call. A contract test asserts CJS and SDK agree on legacy-shape fixtures. (#3523) From 75dffc80692c2df557e8f104b51fab7e926fd812 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 14 May 2026 19:44:01 -0400 Subject: [PATCH 4/5] fix: address branching strategy review findings --- .changeset/sunny-pandas-dance.md | 2 +- get-shit-done/bin/lib/core.cjs | 10 ++- ...config-branching-strategy-warning.test.cjs | 81 +++++++++++++++++-- 3 files changed, 85 insertions(+), 8 deletions(-) diff --git a/.changeset/sunny-pandas-dance.md b/.changeset/sunny-pandas-dance.md index 782c550d0..289ea4f4e 100644 --- a/.changeset/sunny-pandas-dance.md +++ b/.changeset/sunny-pandas-dance.md @@ -1,5 +1,5 @@ --- type: Fixed -pr: 3523 +pr: 3527 --- **Top-level `branching_strategy` in `.planning/config.json` no longer triggers a false "unknown config key" warning** — `loadConfig` in `core.cjs` actively read the key via its nested fallback (`git.branching_strategy`) but the `KNOWN_TOP_LEVEL` allowlist was built from dot-notation paths via `.split('.')[0]`, turning `'git.branching_strategy'` into `'git'` instead of `'branching_strategy'`. The fix adds a self-healing on-disk migration (mirroring the existing `multiRepo → planning.sub_repos` precedent): on first `loadConfig`, the top-level key is grafted into `git.branching_strategy` and the stale top-level entry is removed. A module-level deduplication Set also prevents the same unknown-key warning from appearing twice when `loadConfig` is invoked multiple times in a single CLI call. A contract test asserts CJS and SDK agree on legacy-shape fixtures. (#3523) diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 6454da631..c014de351 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -348,6 +348,14 @@ function loadConfig(cwd, options = {}) { const raw = platformReadSync(rootConfigPath); if (raw === null) throw new Error('missing'); rootParsed = JSON.parse(raw); + if (Object.prototype.hasOwnProperty.call(rootParsed, 'branching_strategy')) { + if (!rootParsed.git) rootParsed.git = {}; + if (rootParsed.git.branching_strategy === undefined) { + rootParsed.git.branching_strategy = rootParsed.branching_strategy; + } + delete rootParsed.branching_strategy; + try { platformWriteSync(rootConfigPath, JSON.stringify(rootParsed, null, 2)); } catch {} + } } catch { // Root config missing or unparseable — workstream config stands alone } @@ -407,7 +415,7 @@ function loadConfig(cwd, options = {}) { // The nested value wins if already set (matches SDK mergeDefaults precedence, PR #3116). if (Object.prototype.hasOwnProperty.call(fileData, 'branching_strategy')) { if (!fileData.git) fileData.git = {}; - if (!fileData.git.branching_strategy) { + if (fileData.git.branching_strategy === undefined) { fileData.git.branching_strategy = fileData.branching_strategy; } delete fileData.branching_strategy; diff --git a/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs b/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs index 4ffc84ed3..611901891 100644 --- a/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs +++ b/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs @@ -57,11 +57,11 @@ const TEST_ENV_BASE = { * Run gsd-tools and return { stdout, stderr, status }. * Always captures stderr even when exit code is 0. */ -function runWithStderr(args, cwd) { +function runWithStderr(args, cwd, env = {}) { const result = spawnSync(process.execPath, [TOOLS_PATH, ...args], { cwd, encoding: 'utf-8', - env: { ...process.env, ...TEST_ENV_BASE }, + env: { ...process.env, ...TEST_ENV_BASE, ...env }, }); return { stdout: result.stdout || '', @@ -126,6 +126,11 @@ describe('bug-3523 — no warning for legacy top-level branching_strategy', () = // After migration write-back, config-get should find git.branching_strategy. const result = runWithStderr(['config-get', 'git.branching_strategy'], tmpDir); + assert.equal( + result.status, + 0, + `config-get command must succeed — exit status ${result.status}, stderr: ${result.stderr}` + ); assert.equal( result.stderr.trim(), '', @@ -152,7 +157,7 @@ describe('bug-3523 — double-emission reduced to single-emission', () => { // Use a key that IS genuinely unknown (not branching_strategy, which is now // fixed) to verify the deduplication guard works for other keys too. // We verify that the count of warning lines for a single unknown key is - // at most 1 — not 2 — even if loadConfig is invoked twice internally. + // exactly once — not zero and not two — even if loadConfig is invoked twice internally. tmpDir = createTempProject('gsd-3523-dedup-'); const configPath = path.join(tmpDir, '.planning', 'config.json'); fs.writeFileSync( @@ -171,9 +176,10 @@ describe('bug-3523 — double-emission reduced to single-emission', () => { .split('\n') .filter(l => l.includes('__gsd3523_dedup_sentinel__')); - assert.ok( - warningLines.length <= 1, - `Unknown-key warning must appear at most once per process invocation — ` + + assert.equal( + warningLines.length, + 1, + `Unknown-key warning must appear exactly once per process invocation — ` + `appeared ${warningLines.length} times. stderr:\n${result.stderr}` ); }); @@ -251,6 +257,64 @@ describe('bug-3523 — option 3 on-disk migration of branching_strategy', () => 'top-level branching_strategy should be removed even when git.branching_strategy already set' ); }); + + test('workstream load also self-heals legacy root branching_strategy', () => { + tmpDir = createTempProject('gsd-3523-workstream-root-'); + const rootConfigPath = path.join(tmpDir, '.planning', 'config.json'); + const workstreamDir = path.join(tmpDir, '.planning', 'workstreams', 'alpha'); + fs.mkdirSync(workstreamDir, { recursive: true }); + fs.writeFileSync( + rootConfigPath, + JSON.stringify({ + branching_strategy: 'phase', + git: { base_branch: 'main' }, + }, null, 2), + 'utf-8' + ); + fs.writeFileSync( + path.join(workstreamDir, 'config.json'), + JSON.stringify({ workflow: { tdd: true } }, null, 2), + 'utf-8' + ); + + const triggerResult = runWithStderr(['resolve-model', 'planner'], tmpDir, { + GSD_WORKSTREAM: 'alpha', + }); + + assert.equal( + triggerResult.status, + 0, + `workstream load command must succeed — exit status ${triggerResult.status}, stderr: ${triggerResult.stderr}` + ); + assert.equal( + triggerResult.stderr.trim(), + '', + `No warning should fire while migrating root config for a workstream — got: ${triggerResult.stderr}` + ); + + const onDisk = JSON.parse(fs.readFileSync(rootConfigPath, 'utf-8')); + assert.equal( + onDisk.git?.branching_strategy, + 'phase', + 'Expected root config.json to persist git.branching_strategy after workstream load' + ); + assert.equal( + onDisk.branching_strategy, + undefined, + 'Expected root config.json to remove top-level branching_strategy after workstream load' + ); + + const rootResult = runWithStderr(['config-get', 'git.branching_strategy'], tmpDir); + assert.equal( + rootResult.status, + 0, + `root config-get command must succeed after workstream migration — exit status ${rootResult.status}, stderr: ${rootResult.stderr}` + ); + assert.ok( + rootResult.stdout.includes('phase'), + `Expected migrated root git.branching_strategy to be 'phase' but got: ${rootResult.stdout}` + ); + }); }); // ─── Test 4: CJS↔SDK contract parity ──────────────────────────────────────── @@ -301,6 +365,11 @@ describe('bug-3523 — CJS↔SDK contract: both agree on legacy branching_strate // matching what the SDK's mergeDefaults would compute. const result = runWithStderr(['config-get', 'git.branching_strategy'], tmpDir); + assert.equal( + result.status, + 0, + `config-get command must succeed — exit status ${result.status}, stderr: ${result.stderr}` + ); assert.equal( result.stderr.trim(), '', From 7c5adeaad134ccebcee2b4e1f0147157aa9510ea Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 14 May 2026 19:47:48 -0400 Subject: [PATCH 5/5] test: allow runtime output assertions --- .../bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs b/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs index 611901891..00562212b 100644 --- a/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs +++ b/tests/bug-3523-cjs-loadconfig-branching-strategy-warning.test.cjs @@ -1,5 +1,7 @@ 'use strict'; +// allow-test-rule: validates runtime CLI stdout/stderr warning behavior, not source grep + /** * Regression tests for #3523 — CJS loadConfig must not emit a false * "unknown config key(s)" warning for `branching_strategy` when that key