* test(#2702): failing-first regression for workstream config-get root inheritance * fix(#2702): workstream config-get inherits absent keys from root config * fix(#2702): root inheritance wins over --default; gate on GSD_WORKSTREAM not path (review) * test(#2702): make GSD_PROJECT test exit-safe (--default sentinel, no error path) * docs(changeset): #2702 workstream config-get root inheritance * docs(changeset): backfill #2702 PR number to 2833
This commit is contained in:
Tom Boucher
5
.changeset/calm-bears-travel.md
Normal file
5
.changeset/calm-bears-travel.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2833
|
||||
---
|
||||
**Workstream-scoped config reads now inherit from the project root config** — `config-get` under an active workstream (`GSD_WORKSTREAM`) now resolves a key absent from the workstream's own config to the project-root value before falling back to schema defaults, instead of reporting 'Key not found'. A workstream config still overrides root for any key it sets; root only fills gaps. Previously a key set only at root was silently lost under a workstream, causing shipped workflow boolean guards (e.g. use_worktrees, plan_review_convergence) to apply their hardcoded fallback and silently invert the user's setting.
|
||||
@@ -18,7 +18,7 @@ const { CONFIG_DEFAULTS } = configLoader;
|
||||
import { platformWriteSync, platformEnsureDir } from './shell-command-projection.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import planningWorkspace = require('./planning-workspace.cjs');
|
||||
const { planningDir, withPlanningLock } = planningWorkspace;
|
||||
const { planningDir, planningRoot, withPlanningLock } = planningWorkspace;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import modelProfiles = require('./model-profiles.cjs');
|
||||
const { VALID_PROFILES, getAgentToModelMapForProfile, formatAgentToModelMapAsTable } = modelProfiles;
|
||||
@@ -965,10 +965,15 @@ function cmdConfigGet(cwd: string, keyPath: string | undefined, raw: boolean, de
|
||||
try {
|
||||
if (fs.existsSync(configPath)) {
|
||||
config = JSON.parse(fs.readFileSync(configPath, 'utf-8')) as Record<string, unknown>;
|
||||
} else if (hasDefault) {
|
||||
emitResolvedDefault(kp, defaultValue, raw);
|
||||
return;
|
||||
} else {
|
||||
// #2702: when a workstream is active and has no config.json of its own, fall
|
||||
// back to the project ROOT config first — a key the user configured at root is
|
||||
// a real, present value and must inherit (per #1893: a present key wins over
|
||||
// --default). Only when root also misses do --default / schema default apply.
|
||||
// (When no workstream is active, resolveFromRootConfig is a no-op: same file.)
|
||||
const rootVal = resolveFromRootConfig(cwd, kp);
|
||||
if (rootVal.found) { emitResolvedDefault(kp, rootVal.value, raw); return; }
|
||||
if (hasDefault) { emitResolvedDefault(kp, defaultValue, raw); return; }
|
||||
const sd = resolveSchemaDefault(cwd, kp);
|
||||
if (sd.found) { emitResolvedDefault(kp, sd.value, raw); return; }
|
||||
error('No config.json found at ' + configPath, ERROR_REASON.CONFIG_NO_FILE);
|
||||
@@ -983,6 +988,9 @@ function cmdConfigGet(cwd: string, keyPath: string | undefined, raw: boolean, de
|
||||
let current: unknown = config;
|
||||
for (const key of keys) {
|
||||
if (current === undefined || current === null || typeof current !== 'object') {
|
||||
// #2702: root-config inheritance before --default / schema default (see above).
|
||||
const rootVal = resolveFromRootConfig(cwd, kp);
|
||||
if (rootVal.found) { emitResolvedDefault(kp, rootVal.value, raw); return; }
|
||||
if (hasDefault) { emitResolvedDefault(kp, defaultValue, raw); return; }
|
||||
const sd = resolveSchemaDefault(cwd, kp);
|
||||
if (sd.found) { emitResolvedDefault(kp, sd.value, raw); return; }
|
||||
@@ -1003,6 +1011,9 @@ function cmdConfigGet(cwd: string, keyPath: string | undefined, raw: boolean, de
|
||||
}
|
||||
|
||||
if (current === undefined) {
|
||||
// #2702: root-config inheritance before --default / schema default (see above).
|
||||
const rootVal = resolveFromRootConfig(cwd, kp);
|
||||
if (rootVal.found) { emitResolvedDefault(kp, rootVal.value, raw); return; }
|
||||
if (hasDefault) { emitResolvedDefault(kp, defaultValue, raw); return; }
|
||||
const sd = resolveSchemaDefault(cwd, kp);
|
||||
if (sd.found) { emitResolvedDefault(kp, sd.value, raw); return; }
|
||||
@@ -1020,6 +1031,51 @@ function cmdConfigGet(cwd: string, keyPath: string | undefined, raw: boolean, de
|
||||
output(current, raw, String(current));
|
||||
}
|
||||
|
||||
/**
|
||||
* #2702: resolve a dot-notation key against the project ROOT config
|
||||
* (`.planning/config.json`), ignoring any active workstream scope. Returns
|
||||
* `{found:false}` when the root config is absent, unparseable, or does not
|
||||
* contain the key. This is the inheritance rung `cmdConfigGet` was missing —
|
||||
* when a workstream's own config doesn't set a key, the project root value
|
||||
* must show through (workstream overrides root; it never fully replaces it),
|
||||
* exactly as `loadConfigResolved`'s root+workstream merge already does for
|
||||
* every other config consumer. No-op (found:false) when no workstream is
|
||||
* active, because `planningDir === planningRoot` and the caller already read
|
||||
* that file directly.
|
||||
*/
|
||||
function resolveFromRootConfig(cwd: string, kp: string): { found: boolean; value: unknown } {
|
||||
// Only meaningful when a workstream is active (GSD_WORKSTREAM set) — that is what
|
||||
// redirects planningDir away from root AND what loadConfigResolved gates root-reading
|
||||
// on. Gating on `process.env.GSD_WORKSTREAM` (not on a planningDir !== planningRoot
|
||||
// path inequality) avoids a false trigger under GSD_PROJECT alone, where planningDir
|
||||
// diverges from planningRoot without a workstream and loadConfigResolved does NOT
|
||||
// inherit root — matching the runtime's own `if (ws)` gate keeps the two surfaces
|
||||
// from diverging on the project-scoped (non-workstream) case.
|
||||
if (!process.env['GSD_WORKSTREAM']) return { found: false, value: undefined };
|
||||
const root = planningRoot(cwd);
|
||||
const rootConfigPath = path.join(root, 'config.json');
|
||||
let rootConfig: Record<string, unknown>;
|
||||
try {
|
||||
if (!fs.existsSync(rootConfigPath)) return { found: false, value: undefined };
|
||||
rootConfig = JSON.parse(fs.readFileSync(rootConfigPath, 'utf-8')) as Record<string, unknown>;
|
||||
} catch {
|
||||
// Unparseable root config → don't inherit (do not let a corrupt root file
|
||||
// change config-get's verdict). Fall through to schema default / error.
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
let current: unknown = rootConfig;
|
||||
for (const key of kp.split('.')) {
|
||||
if (current === undefined || current === null || typeof current !== 'object') {
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
current = Object.prototype.hasOwnProperty.call(current, key)
|
||||
? (current as Record<string, unknown>)[key]
|
||||
: undefined;
|
||||
}
|
||||
if (current === undefined) return { found: false, value: undefined };
|
||||
return { found: true, value: current };
|
||||
}
|
||||
|
||||
/**
|
||||
* Command to set the model profile in the config file.
|
||||
*
|
||||
|
||||
@@ -1376,3 +1376,182 @@ test('runCli treats jsonErrors=false as an explicit human-formatter path', (t) =
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// ─── #2702: workstream-scoped config-get inherits from root config ──────────
|
||||
//
|
||||
// When GSD_WORKSTREAM is set, planningDir(cwd) points at .planning/workstreams/<ws>/.
|
||||
// cmdConfigGet used to read ONLY that file, so a key configured at the project root
|
||||
// (.planning/config.json) but absent from the workstream's own config was reported
|
||||
// "Key not found" — silently inverting boolean workflow guards. The fix adds a
|
||||
// root-config inheritance rung: workstream overrides root, but root fills gaps.
|
||||
|
||||
describe('#2702: workstream config-get inherits from root config', () => {
|
||||
let tmpDir;
|
||||
let rootPlanningDir;
|
||||
let wsPlanningDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-config-ws-2702-'));
|
||||
rootPlanningDir = path.join(tmpDir, '.planning');
|
||||
wsPlanningDir = path.join(rootPlanningDir, 'workstreams', 'alpha');
|
||||
fs.mkdirSync(wsPlanningDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
// Run cmdConfigGet with GSD_WORKSTREAM scoped to 'alpha' (so planningDir → ws dir).
|
||||
function runScoped(...args) {
|
||||
const { keyPath, raw, defaultValue } = parseConfigGetArgs(args);
|
||||
const saved = process.env.GSD_WORKSTREAM;
|
||||
process.env.GSD_WORKSTREAM = 'alpha';
|
||||
let out;
|
||||
try {
|
||||
out = captureFdWrite(1, () => {
|
||||
config.cmdConfigGet(tmpDir, keyPath, raw, defaultValue);
|
||||
});
|
||||
} finally {
|
||||
if (saved === undefined) delete process.env.GSD_WORKSTREAM;
|
||||
else process.env.GSD_WORKSTREAM = saved;
|
||||
}
|
||||
return out.trim();
|
||||
}
|
||||
|
||||
function runScopedExpectError(...args) {
|
||||
const { keyPath, raw, defaultValue } = parseConfigGetArgs(args);
|
||||
const saved = process.env.GSD_WORKSTREAM;
|
||||
process.env.GSD_WORKSTREAM = 'alpha';
|
||||
const origExit = process.exit;
|
||||
const origWriteSync = fs.writeSync;
|
||||
io.setJsonErrorMode(true);
|
||||
let exitCode;
|
||||
let stderr = '';
|
||||
fs.writeSync = (fd, ...rest) => {
|
||||
if (fd !== 2) return origWriteSync.call(fs, fd, ...rest);
|
||||
stderr += String(rest[0]);
|
||||
return Buffer.byteLength(String(rest[0]));
|
||||
};
|
||||
process.exit = (code) => { exitCode = code; throw new _ExitSignal(code); };
|
||||
try {
|
||||
config.cmdConfigGet(tmpDir, keyPath, raw, defaultValue);
|
||||
} catch (e) {
|
||||
if (!(e instanceof _ExitSignal)) throw e;
|
||||
} finally {
|
||||
process.exit = origExit;
|
||||
fs.writeSync = origWriteSync;
|
||||
io.setJsonErrorMode(false);
|
||||
if (saved === undefined) delete process.env.GSD_WORKSTREAM;
|
||||
else process.env.GSD_WORKSTREAM = saved;
|
||||
}
|
||||
const parts = stderr.split('\n').filter(Boolean);
|
||||
let payload = {};
|
||||
try { payload = JSON.parse(parts[parts.length - 1]); } catch { /* human mode */ }
|
||||
return { status: exitCode, reason: payload.reason };
|
||||
}
|
||||
|
||||
function writeRoot(obj) {
|
||||
fs.writeFileSync(path.join(rootPlanningDir, 'config.json'), JSON.stringify(obj));
|
||||
}
|
||||
function writeWs(obj) {
|
||||
fs.writeFileSync(path.join(wsPlanningDir, 'config.json'), JSON.stringify(obj));
|
||||
}
|
||||
|
||||
test('inherits a fail-closed key (workflow.use_worktrees) from root when workstream omits it', () => {
|
||||
writeRoot({ workflow: { use_worktrees: true } });
|
||||
// workstream config exists but does NOT set the key
|
||||
writeWs({ workflow: {} });
|
||||
assert.equal(runScoped('config-get', 'workflow.use_worktrees', '--raw'), 'true');
|
||||
});
|
||||
|
||||
test('inherits a fail-open key (workflow.plan_review_convergence) from root', () => {
|
||||
writeRoot({ workflow: { plan_review_convergence: true } });
|
||||
writeWs({ workflow: {} });
|
||||
assert.equal(runScoped('config-get', 'workflow.plan_review_convergence', '--raw'), 'true');
|
||||
});
|
||||
|
||||
test('inherits a core key (workflow.verifier) from root', () => {
|
||||
writeRoot({ workflow: { verifier: true } });
|
||||
writeWs({ workflow: {} });
|
||||
assert.equal(runScoped('config-get', 'workflow.verifier', '--raw'), 'true');
|
||||
});
|
||||
|
||||
test('workstream config overrides root value for a key it sets', () => {
|
||||
writeRoot({ workflow: { use_worktrees: true } });
|
||||
writeWs({ workflow: { use_worktrees: false } });
|
||||
assert.equal(runScoped('config-get', 'workflow.use_worktrees', '--raw'), 'false');
|
||||
});
|
||||
|
||||
test('still errors on a key absent from workstream, root, and schema', () => {
|
||||
writeWs({ workflow: {} });
|
||||
writeRoot({ workflow: {} });
|
||||
const { status, reason } = runScopedExpectError('config-get', 'workflow.totally_absent', '--raw');
|
||||
assert.notEqual(status, 0, 'a key absent everywhere must error');
|
||||
assert.equal(reason, io.ERROR_REASON.CONFIG_KEY_NOT_FOUND);
|
||||
});
|
||||
|
||||
test('--default is ignored when the key is inherited from root', () => {
|
||||
writeRoot({ workflow: { use_worktrees: true } });
|
||||
writeWs({ workflow: {} });
|
||||
assert.equal(
|
||||
runScoped('config-get', 'workflow.use_worktrees', '--default', 'false', '--raw'),
|
||||
'true',
|
||||
);
|
||||
});
|
||||
|
||||
test('inherits a nested fail-closed key (workflow.context_coverage_gate) from root', () => {
|
||||
writeRoot({ workflow: { context_coverage_gate: false } });
|
||||
writeWs({ workflow: {} });
|
||||
assert.equal(runScoped('config-get', 'workflow.context_coverage_gate', '--raw'), 'false');
|
||||
});
|
||||
|
||||
test('scoped and unscoped reads return the same string form when workstream does not override', () => {
|
||||
writeRoot({ workflow: { use_worktrees: true } });
|
||||
writeWs({ workflow: {} });
|
||||
const scoped = runScoped('config-get', 'workflow.use_worktrees', '--raw');
|
||||
// Unscoped read: no GSD_WORKSTREAM — reads root directly.
|
||||
const unscoped = captureFdWrite(1, () => {
|
||||
config.cmdConfigGet(tmpDir, 'workflow.use_worktrees', true, undefined);
|
||||
}).trim();
|
||||
assert.equal(scoped, unscoped, 'scoped (inherited) and unscoped reads must agree');
|
||||
assert.equal(scoped, 'true');
|
||||
});
|
||||
|
||||
test('unscoped config-get still reads root value (no regression when not scoped)', () => {
|
||||
writeRoot({ workflow: { use_worktrees: true } });
|
||||
const out = captureFdWrite(1, () => {
|
||||
config.cmdConfigGet(tmpDir, 'workflow.use_worktrees', true, undefined);
|
||||
}).trim();
|
||||
assert.equal(out, 'true');
|
||||
});
|
||||
|
||||
test('inherits from root even when the workstream has no config.json at all', () => {
|
||||
writeRoot({ workflow: { use_worktrees: true } });
|
||||
// No writeWs — workstreams/alpha/config.json does not exist.
|
||||
assert.equal(runScoped('config-get', 'workflow.use_worktrees', '--raw'), 'true');
|
||||
});
|
||||
|
||||
test('GSD_PROJECT alone (no workstream) does not trigger root inheritance — matches loadConfigResolved', () => {
|
||||
// GSD_PROJECT scopes planningDir to .planning/<project>/ but loadConfigResolved
|
||||
// gates root-reading on `if (ws)`, so a project-only read must NOT inherit root.
|
||||
// The fix gates on GSD_WORKSTREAM presence (not path inequality) to match.
|
||||
// A --default is supplied so a non-inheriting read returns a clean sentinel
|
||||
// ('not-inherited') rather than hitting the error/exit path.
|
||||
const projectPlanningDir = path.join(rootPlanningDir, 'myproj');
|
||||
fs.mkdirSync(projectPlanningDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(projectPlanningDir, 'config.json'), JSON.stringify({ workflow: {} }));
|
||||
writeRoot({ workflow: { use_worktrees: true } });
|
||||
const saved = process.env.GSD_PROJECT;
|
||||
process.env.GSD_PROJECT = 'myproj';
|
||||
try {
|
||||
const out = captureFdWrite(1, () => {
|
||||
config.cmdConfigGet(tmpDir, 'workflow.use_worktrees', true, 'not-inherited');
|
||||
}).trim();
|
||||
// No workstream → no root inheritance → returns the --default sentinel, NOT root 'true'.
|
||||
assert.equal(out, 'not-inherited', 'GSD_PROJECT-only must not inherit root (matches loadConfigResolved)');
|
||||
} finally {
|
||||
if (saved === undefined) delete process.env.GSD_PROJECT;
|
||||
else process.env.GSD_PROJECT = saved;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user